Go: Add models for Go 1.27 and expand models for strings package - #22534
Open
owen-mc wants to merge 8 commits into
Open
Go: Add models for Go 1.27 and expand models for strings package#22534owen-mc wants to merge 8 commits into
strings package#22534owen-mc wants to merge 8 commits into
Conversation
Contributor
Click to show differences in coveragegoGenerated file changes for go
- `Standard library <https://pkg.go.dev/std>`_,"````, ``archive/*``, ``bufio``, ``bytes``, ``cmp``, ``compress/*``, ``container/*``, ``context``, ``crypto``, ``crypto/*``, ``database/*``, ``debug/*``, ``embed``, ``encoding``, ``encoding/*``, ``errors``, ``expvar``, ``flag``, ``fmt``, ``go/*``, ``hash``, ``hash/*``, ``html``, ``html/*``, ``image``, ``image/*``, ``index/*``, ``io``, ``io/*``, ``log``, ``log/*``, ``maps``, ``math``, ``math/*``, ``mime``, ``mime/*``, ``net``, ``net/*``, ``os``, ``os/*``, ``path``, ``path/*``, ``plugin``, ``reflect``, ``reflect/*``, ``regexp``, ``regexp/*``, ``slices``, ``sort``, ``strconv``, ``strings``, ``sync``, ``sync/*``, ``syscall``, ``syscall/*``, ``testing``, ``testing/*``, ``text/*``, ``time``, ``time/*``, ``unicode``, ``unicode/*``, ``unsafe``, ``weak``",52,625,127
+ `Standard library <https://pkg.go.dev/std>`_,"````, ``archive/*``, ``bufio``, ``bytes``, ``cmp``, ``compress/*``, ``container/*``, ``context``, ``crypto``, ``crypto/*``, ``database/*``, ``debug/*``, ``embed``, ``encoding``, ``encoding/*``, ``errors``, ``expvar``, ``flag``, ``fmt``, ``go/*``, ``hash``, ``hash/*``, ``html``, ``html/*``, ``image``, ``image/*``, ``index/*``, ``io``, ``io/*``, ``log``, ``log/*``, ``maps``, ``math``, ``math/*``, ``mime``, ``mime/*``, ``net``, ``net/*``, ``os``, ``os/*``, ``path``, ``path/*``, ``plugin``, ``reflect``, ``reflect/*``, ``regexp``, ``regexp/*``, ``slices``, ``sort``, ``strconv``, ``strings``, ``sync``, ``sync/*``, ``syscall``, ``syscall/*``, ``testing``, ``testing/*``, ``text/*``, ``time``, ``time/*``, ``unicode``, ``unicode/*``, ``unsafe``, ``weak``",52,674,127
- Totals,,688,1085,1580
+ Totals,,688,1134,1580
- bytes,,,44,,,,,,,,,,,,,,,,,,,,,,,44,
+ bytes,,,45,,,,,,,,,,,,,,,,,,,,,,,45,
- database/sql,30,18,12,,,,,,,,,,,,30,,,,,,18,,,,,12,
+ database/sql,30,18,14,,,,,,,,,,,,30,,,,,,18,,,,,14,
- encoding,,,81,,,,,,,,,,,,,,,,,,,,,,,81,
+ encoding,,,112,,,,,,,,,,,,,,,,,,,,,,,112,
- net,2,16,100,,,,,,1,,,,,,,,1,,,,,,,16,,100,
+ net,2,16,102,,,,,,1,,,,,,,,1,,,,,,,16,,102,
- strings,,,34,,,,,,,,,,,,,,,,,,,,,,,34,
+ strings,,,47,,,,,,,,,,,,,,,,,,,,,,,46,1 |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The iterator summary is ineffective for yielded values, and several new or refined models lack appropriate flow tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 4
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
go/ql/lib/ext/encoding.json.jsontext.model.yml — No test under go/ql/test imports or calls encoding/json/jsontext, so none of the new summary… |
|
go/ql/lib/ext/encoding.json.jsontext.model.yml — Pointer.Tokens returns an iter.Seq[string], so this edge taints only the returned iterator… |
|
go/ql/lib/ext/strings.model.yml — These summaries now target only the returned slice elements, but the existing tests still pass the… |
|
go/ql/lib/semmle/go/frameworks/stdlib/EncodingJson.qll — The new reverse-flow branch has no jsontext.NewEncoder test. The existing encoding/json branch… |
What changed in this PR
Adds Go 1.27 standard-library data-flow models and improves strings model precision.
Changes:
- Models new Go 1.27 APIs and
encoding/json/jsontext. - Expands and refines
stringssummaries. - Adds taint-flow fixtures and release notes.
| File | Description |
|---|---|
Strings.go |
Adds and updates strings flow fixtures. |
NetHttp.go |
Tests URL clone models. |
go.mod |
Targets Go 1.27. |
DatabaseSqlDriver.go |
Tests ScanColumn. |
DatabaseSql.go |
Tests ConvertAssign. |
Bytes.go |
Tests bytes.CutLast. |
EncodingJson.qll |
Models jsontext encoder reverse flow. |
strings.model.yml |
Expands strings summaries. |
net.url.model.yml |
Adds URL clone summaries. |
encoding.json.jsontext.model.yml |
Adds jsontext summaries. |
database.sql.model.yml |
Models ConvertAssign. |
database.sql.driver.model.yml |
Models ScanColumn. |
bytes.model.yml |
Models bytes.CutLast. |
2026-09-09-go-1.27-models.md |
Documents analysis improvements. |
Files not reviewed (5)
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Bytes.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSql.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSqlDriver.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/NetHttp.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Strings.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
9 tasks
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
URL clone summaries misuse value-preserving flow, and unrelated C++ and documentation changes should be split out.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (5)
| Severity | Finding |
|---|---|
|
go/ql/lib/ext/net.url.model.yml — value declares a globally value-preserving edge, but URL.Clone allocates and returns a… |
|
go/ql/lib/ext/net.url.model.yml — Values.Clone creates a distinct map, so receiver-to-return flow is not value-preserving. The… |
 |
cpp/ql/lib/ext/Boost.Asio.model.yml — The stated PR scope is Go 1.27 and strings, but this starts an independent Boost.Asio… |
|
docs/codeql/index.html — This documentation asset update, together with the added CodeQL 2.27 release pages, is unrelated to… |
|
go/ql/lib/ext/strings.model.yml — The separator flow introduced on this line is not exercised: the current Join fixture only… |
Issues resolved since last review (4)
| Severity | Finding |
|---|---|
|
go/ql/lib/semmle/go/frameworks/stdlib/EncodingJson.qll — The new reverse-flow branch has no jsontext.NewEncoder test. The existing encoding/json branch… View resolved comment |
|
go/ql/lib/ext/strings.model.yml — These summaries now target only the returned slice elements, but the existing tests still pass the… View resolved comment |
|
go/ql/lib/ext/encoding.json.jsontext.model.yml — Pointer.Tokens returns an iter.Seq[string], so this edge taints only the returned iterator… View resolved comment |
|
go/ql/lib/ext/encoding.json.jsontext.model.yml — No test under go/ql/test imports or calls encoding/json/jsontext, so none of the new summary… View resolved comment |
Files not reviewed (5)
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Bytes.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSql.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSqlDriver.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/NetHttp.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Strings.go: Generated file
Functions returning an `iter.Seq` are not yet modeled.
drop jsontext Pointer.Tokens model, add jsontext tests, fix strings Fields tests Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Boolean jsontext summaries are missing, and unrelated C++ and documentation changes should be separated.
Review tier: Balanced
Findings: None
Issues resolved since last review (5)
| Severity | Finding |
|---|---|
|
go/ql/lib/ext/strings.model.yml — The separator flow introduced on this line is not exercised: the current Join fixture only… View resolved comment |
|
docs/codeql/index.html — This documentation asset update, together with the added CodeQL 2.27 release pages, is unrelated to… View resolved comment |
|
cpp/ql/lib/ext/Boost.Asio.model.yml — The stated PR scope is Go 1.27 and strings, but this starts an independent Boost.Asio… View resolved comment |
|
go/ql/lib/ext/net.url.model.yml — Values.Clone creates a distinct map, so receiver-to-return flow is not value-preserving. The… View resolved comment |
|
go/ql/lib/ext/net.url.model.yml — value declares a globally value-preserving edge, but URL.Clone allocates and returns a… View resolved comment |
Files not reviewed (5)
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Bytes.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSql.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSqlDriver.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/NetHttp.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Strings.go: Generated file
Previously missed findings (2)
In code that hasn't changed since last review
go/ql/lib/ext/encoding.json.jsontext.model.yml:15
- The scalar-token models skip
jsontext.Bool(bool) TokenandToken.Bool() bool, although the same constructor/accessor pairs are modeled for strings and every numeric token type. Calls through either boolean API therefore lose taint. Please add both summary rows and corresponding StdlibTaintFlow cases.
docs/codeql/codeql-overview/codeql-changelog/index.rst:14 - Adding the CodeQL 2.27.0 release changelog and toctree entry is unrelated to the PR's stated Go modeling scope. Please exclude the release-documentation changes from this pull request.
owen-mc
force-pushed
the
go/mad/1.27
branch
from
September 10, 2026 21:50
aab52ab to
e009997
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Both net/url deep-copy models are incorrectly marked as globally value-preserving.
Review tier: Balanced
Findings: None
Files not reviewed (5)
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Bytes.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSql.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/DatabaseSqlDriver.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/NetHttp.go: Generated file
- go/ql/test/library-tests/semmle/go/frameworks/StdlibTaintFlow/Strings.go: Generated file
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
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.
bytes.CutLast,database/sql.ConvertAssign,database/sql/driver.RowsColumnScanner.ScanColumn,net/url.URL.Clone,net/url.Values.Cloneandstrings.CutLast.encoding/json/jsontextpackage.stringspackage:strings.Clone,Cut,CutPrefix,CutSuffix,Fields,FieldsFunc, andJoin;strings.Builder.String,Builder.WriteByte, andBuilder.WriteRune;strings.Reader.ReadByteandReader.ReadRune; andstrings.Replacer.ReplaceandReplacer.WriteString.Note that there are some new tests in the
stringspackage that are commented out. They are all functions which return aiter.Seq, which is not supported yet. I intend to do a follow-up PR soon to add support for it.