Uh oh!
There was an error while loading. Please reload this page.
FlowSummaryImpl: Model more source/sink steps as jump steps - #22470
Conversation
001b6cd to
4144ee0Compare4144ee0 to
c0610f0Comparec0610f0 to
78dce11CompareThere was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The unrelated Windows CI-status filtering should be reverted, separated, or documented.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 | misc/scripts/accept-expected-changes-from-ci.py — This changes contributor tooling outside the PR's stated flow-summary scope: Windows language-test… |
What changed in this PR
Refines flow-summary path graphs so callback and parameter nodes remain visible.
Changes:
- Reclassifies source/sink boundary steps as jump steps where appropriate.
- Updates Rust and C++ path expectations.
- Modifies CI expected-output tooling, including Windows status filtering.
| File | Description |
|---|---|
shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll | Refines source/sink step classification. |
rust/ql/test/query-tests/security/CWE-918/RequestForgery.expected | Updates request-forgery paths. |
rust/ql/test/query-tests/security/CWE-117/LogInjection.expected | Updates log-injection paths. |
rust/ql/test/query-tests/security/CWE-079/warp/XSS.expected | Updates Warp XSS paths. |
rust/ql/test/query-tests/security/CWE-079/axum/XSS.expected | Updates Axum XSS paths. |
rust/ql/test/query-tests/security/CWE-079/actix/XSS.expected | Updates Actix XSS paths. |
rust/ql/test/library-tests/dataflow/sources/web_frameworks/InlineFlow.expected | Records intermediate web-framework nodes. |
rust/ql/test/library-tests/dataflow/sources/database/InlineFlow.expected | Records intermediate database callback nodes. |
rust/ql/test/library-tests/dataflow/models/models.expected | Updates modeled-flow paths. |
misc/scripts/accept-expected-changes-from-ci.py | Changes log retrieval and Windows filtering. |
cpp/ql/test/library-tests/dataflow/external-models/flow.expected | Updates external-model paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| lang_test_failures: List[GithubStatus] = list() | ||
| for status in newest_status.values(): | ||
| if " Language Tests" in status.context or status.context in supported_internal_status_language_test_names: | ||
| if " Language Tests" in status.context and not " Language Tests Windows" in status.context or status.context in supported_internal_status_language_test_names: |
There was a problem hiding this comment.
This probably isn't as big as a deal as Copilot is making it out to be (since this is just a script we can run locally). However, what's the motivation for excluding language tests on Windows here (at least .. I think that's what's going on)?
There was a problem hiding this comment.
It was causing issues for me when running the script on this PR where it failed to generate diffs related to windows file paths. Not sure exactly why though...
78dce11 to
d64ad79Compare
MathiasVP
left a comment
There was a problem hiding this comment.
The QL changes (and expected) looks good to me! My only question is that change to the Python script.
Uh oh!
There was an error while loading. Please reload this page.
Modeling more source/sink steps as jump steps instead of local steps means that we avoid skipping over nodes that we would like to see in the path graph. For example, in
we would previously get a direct edge
C -> B, but now we instead get two edgesC -> AandA -> B.