JS: Generate legacy flow steps for all flow summaries#20169
Merged
hvitved merged 1 commit intogithub:mainfrom Aug 6, 2025
Merged
JS: Generate legacy flow steps for all flow summaries#20169hvitved merged 1 commit intogithub:mainfrom
hvitved merged 1 commit intogithub:mainfrom
Conversation
hvitved
force-pushed
the
javascript/legacy-summary-steps
branch
from
4547d9f to
eb3c054
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that legacy data flow/taint tracking libraries receive flow steps for all flow summaries, not just those that are unsupported by the new flow summary system. Previously, legacy flow steps were only generated for summaries that couldn't be handled as proper flow summaries, but legacy libraries don't use SummarizedCallables at all and need steps for all summaries.
Key Changes:
- Modified the logic to generate legacy flow steps for all flow summaries instead of only unsupported ones
- Added new legacy flow step classes that handle all summary-based steps
- Updated the test expectations to reflect that legacy and new data flow libraries now have consistent behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll | Added logic to generate legacy flow steps for all summaries and created separate legacy step classes |
| javascript/ql/test/library-tests/frameworks/data/test.expected | Updated test expectations to remove differences between legacy and new data flow libraries |
Napalys
approved these changes
Aug 6, 2025
Contributor
Napalys
left a comment
Napalys
left a comment
There was a problem hiding this comment.
Thank you very much for tackling this! ❤️ I had tried previously looking into it, but wasn't able to find a solution. This looks good to me!
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.
#19445 follow up: The legacy data flow/taint tracking libraries do not use
SummarizedCallables, so we still need to generate local flow steps for all summaries.