Java/C++/Go/C#: Share parts of ExternalFlow.qll #21011
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aschackmull
force-pushed
the
mad/shared-externalflow
branch
3 times, most recently
from
cabfbd0 to
2788f38
Compare
aschackmull
force-pushed
the
mad/shared-externalflow
branch
from
2788f38 to
1bcca6b
Compare
aschackmull
force-pushed
the
mad/shared-externalflow
branch
from
1bcca6b to
4b2e8c0
Compare
There was a problem hiding this comment.
Pull request overview
This PR extracts common Models as Data (MaD) logic from language-specific ExternalFlow.qll files (Java, C++, Go, C#) into a shared library shared/mad/codeql/mad/static/MaD.qll. The refactoring eliminates duplicated code while maintaining language-specific customization through module signatures.
Key changes:
- Created a shared
ModelsAsDatamodule with extensible signatures for language-specific implementations - Unified handling of model predicates (source, sink, summary, barrier, barrier guard, neutral) and model coverage calculations
- Added barrier and barrier guard model support to Go, C#, and C++ through new extensible predicates
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shared/mad/codeql/mad/static/MaD.qll | New shared module providing signatures and generic logic for MaD models across languages |
| java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll | Refactored to use shared MaD module, removing duplicated predicates |
| java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll | Added Extensions module implementing SharedMaD::ExtensionsSig |
| java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll | Updated to use model parameter instead of madId for tracking |
| go/ql/lib/semmle/go/dataflow/ExternalFlow.qll | Refactored to use shared MaD module |
| go/ql/lib/semmle/go/dataflow/internal/ExternalFlowExtensions.qll | Added barrier models and Extensions module implementation |
| go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll | Updated to use model parameter for consistency |
| go/ql/lib/ext/empty.model.yml | Added barrier and barrier guard model extensibles |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll | Refactored to use shared MaD module |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlowExtensions.qll | Added barrier models and Extensions module implementation |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll | Updated to use model parameter |
| csharp/ql/lib/ext/empty.model.yml | Added barrier and barrier guard model extensibles |
| cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll | Refactored to use shared MaD module with custom summaryModel override for @ expansion |
| cpp/ql/lib/semmle/code/cpp/dataflow/internal/ExternalFlowExtensions.qll | Added barrier, neutral models and Extensions module implementation |
| cpp/ql/lib/ext/empty.model.yml | Added barrier and barrier guard model extensibles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shared/mad/codeql/mad/static/MaD.qll
Outdated
| none() | ||
| } | ||
|
|
||
| /** Get the separator used between namespace segments. */ |
There was a problem hiding this comment.
The comment should use "Gets" instead of "Get" to follow standard QLDoc conventions for predicate documentation.
shared/mad/codeql/mad/static/MaD.qll
Outdated
| /** | ||
| * Holds if the package `package` is part of the group `group`. | ||
| */ | ||
| predicate packageGrouping(string group, string package); |
There was a problem hiding this comment.
Should we make this namespaceGrouping? And for go we can also support the existing packageGrouping for a bit as we transition over to namespaceGrouping?
There was a problem hiding this comment.
Yeah, we probably should - I guess we can do two parallel extensible predicates for go as the transition solution.
There was a problem hiding this comment.
I've opted for just changing the shared part for now. Then a switch for Go with deprecation and all that can be a separate PR.
shared/mad/codeql/mad/static/MaD.qll
Outdated
There was a problem hiding this comment.
Dynamic languages have ModelsAsData.qll; I think it would be better to spell it out here as well.
aschackmull
added
the
no-change-note-required
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
Go LGTM. One optional comment about a parameter name.
| summaryModel(p, _, _, _, _, _, _, _, _, _, _) | ||
| ) | ||
| bindingset[p] | ||
| string cleanNamespace(string p) { |
There was a problem hiding this comment.
Optional: Could rename parameter to ns.
3 participants
The ExternalFlow.qll files for Java/C++/Go/C# contain a mix of copy-pasted code and language-specific code. This PR attempts to pull shared bits and pieces into a shared library. Commit-by-commit review is encouraged.