Repository navigation
fix(security): remove no-op identity replacement in errorInspector path helper (#1642) - #1660
Merged
Merged
Conversation
…th helper (Fixes #1642) The .replace(/\]\[/g, "][") at helpers.ts:194 replaced "][" with itself, a no-op flagged by CodeQL (js/identity-replacement, alert #23). It carried the comment "Clean up any double brackets that might have been created", but adjacent brackets like "[0][1]" are valid lodash nested-access syntax and are exactly what the downstream lodash/fp/path consumer expects. The cleanup it implied was never needed, and the line never changed anything -- all existing tests pass with it removed. Removed the dead replacement and clarified the comment. Added tests covering markers sitting directly between brackets (the case the line pretended to guard) plus the empty/undefined path cases.
c4lm
approved these changes
Sep 24, 2026
nthmost-orkes
added a commit
that referenced
this pull request
Sep 25, 2026
…th helper (Fixes #1642) (#1660) The .replace(/\]\[/g, "][") at helpers.ts:194 replaced "][" with itself, a no-op flagged by CodeQL (js/identity-replacement, alert #23). It carried the comment "Clean up any double brackets that might have been created", but adjacent brackets like "[0][1]" are valid lodash nested-access syntax and are exactly what the downstream lodash/fp/path consumer expects. The cleanup it implied was never needed, and the line never changed anything -- all existing tests pass with it removed. Removed the dead replacement and clarified the comment. Added tests covering markers sitting directly between brackets (the case the line pretended to guard) plus the empty/undefined path cases. (cherry picked from commit b3b9ad3)
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.
Fixes #1642 (CodeQL alert #23, rule
js/identity-replacement, sub-issue of #1010).What was wrong
jakatraPathToPropertyPathinui-next/src/pages/definition/errorInspector/state/helpers.tshad:This replaces
][with][— a no-op. CodeQL flags identity replacements because they usually mean an intended transformation isn't happening.Why removing it is the correct fix
The helper converts a Jakarta bean-validation path (e.g.
update.workflowDefs[0].tasks[1].forkTasks[0].<list element>[0]) into a lodash path string that's then fed tolodash/fp/path. After the<list element>/<map value>markers are stripped and.[is collapsed to[, you can end up with adjacent brackets like[0][0].That's not a defect to clean up —
[0][0]is valid lodash nested-access syntax and is exactly what the downstream_path()call expects. The]["cleanup" was guarding against a problem that doesn't exist, and since it replaced the substring with itself it never did anything anyway. Every existing test passes unchanged with the line removed.So the honest resolution is to delete the dead line and clarify the comment, rather than invent a transformation that would actually break correct output.
Changes
.replace(/\]\[/g, "][")and its misleading comment; noted on the adjacent.[cleanup that nested brackets are intentionally preserved.<list element>marker sitting directly between brackets (the case the removed line pretended to guard) ->[1].forkTasks[0][0]<map value>marker between brackets ->[1].decisionCases[switch_case][0]undefinedpaths ->""All
jakatraPathToPropertyPathtests pass (7 total). Prettier and eslint clean.