Add combined changelogs for 2.15.3 and backfill historic versions#14876
Add combined changelogs for 2.15.3 and backfill historic versions#14876turbo merged 1 commit intocodeql-cli-2.15.3from
Conversation
felicitymay
left a comment
There was a problem hiding this comment.
The local preview of these files looks good to me. I've added a few minor comments on the article with the 2.15.3 release notes, but nothing major.
| * Lower the severity of log-injection to medium. | ||
| * Increase the severity of XSS to high. |
There was a problem hiding this comment.
I think we should be explicit here that we're talking about security_severity. Also what changes are we reflecting - did we realize that we'd misclassified them, or did something change about the way we calculate this?
| * Lower the severity of log-injection to medium. | |
| * Increase the severity of XSS to high. | |
| * Lower the security severity of log-injection to medium. | |
| * Increase the security severity of XSS to high. |
| * The predicate :code:`UnboundGeneric::getName` now prints the number of type parameters as a :code:` `N` suffix, instead of a :code:`<,...,>` suffix. For example, the unbound generic type | ||
| :code:`System.Collections.Generic.IList<T>` is printed as :code:`IList`1` instead of :code:`IList<>`. | ||
| * The predicates :code:`hasQualifiedName`, :code:`getQualifiedName`, and :code:`getQualifiedNameWithTypes` have been deprecated, and are instead replaced by :code:`hasFullyQualifiedName`, :code:`getFullyQualifiedName`, and :code:`getFullyQualifiedNameWithTypes`, respectively. The new predicates use the same format for unbound generic types as mentioned above. | ||
| * These changes also affect models-as-data rows that refer to a field or a property belonging to a generic type. For example, instead of writing |
There was a problem hiding this comment.
I don't believe that we've introduced users to the "models-as-data" terminology yet. We also haven't shipped CodeQL model packs for languages other than Java yet. Not sure that this is a user-facing detail 🤔
Also, I thought that our main intention was to get people generating model packs using the VS Code extension not writing things by hand.
There was a problem hiding this comment.
This is a slightly grey area because the 'models as data' in the C# are clearly user-facing but we won't officially support CodeQL model packs for C# until the model editor works for C#. However this is a minor analysis change that only the most curious of users will engage with so I think this changenote is ok.
cc @coadaflorin just in case you have a different view.
There was a problem hiding this comment.
My point was mostly that if we haven't published information to users about these rows, then generally they shouldn't have a change note.
There was a problem hiding this comment.
But happy to leave it there if you think it's useful to users.
There was a problem hiding this comment.
If writing MaD specifications for C# is not even (un)officially supported, we can drop the change note.
There was a problem hiding this comment.
However this is a minor analysis change that only the most curious of users will engage with so I think this changenote is ok.
FTR, I also think it's OK to have here. If we get questions about it, we can re-evaluate.
There was a problem hiding this comment.
Thanks. Something else seems to be off: There is missing something after For example, instead of writing in https://codeql.github.com/docs/codeql-overview/codeql-changelog/codeql-cli-2.15.3/#id7.
There was a problem hiding this comment.
Can you point us at the original source file, so we can work out what's missing and what went wrong with the generator?
There was a problem hiding this comment.
It's https://raw.githubusercontent.com/github/codeql/main/csharp/ql/lib/change-notes/released/0.8.3.md
I found the bug and will fix it next week (in the meantime, another temp fix in the generated files is OK). Turns out this changenote is a great test case Tom 😆
There was a problem hiding this comment.
Fixed as part of #14889 - I discovered a handful of other bugs based on this investigation
| Golang | ||
| """""" | ||
|
|
||
| * Added the `gin cors <https://github.com/gin-contrib/cors>`__ library to the CorsMisconfiguration.ql query |
There was a problem hiding this comment.
This doesn't seem to be how we normally refer to queries. Usually we seem to use the ID, sometimes with the name.
There was a problem hiding this comment.
Agreed. I am happy to change that. But this is actually an experimental query, so I'm not sure it warrants a change note at all. Is that right?
There was a problem hiding this comment.
That's a good point. @turbo - what do you think?
There was a problem hiding this comment.
I'm actually happy for the info to be included, it is a change after all that some users (of the experimental suite) may be affected by. The consistency in how we refer to queries is still good feedback.
|
Thank you for the detailed review. I double checked the issues that looked like possible generator bugs, and confirmed they exist as well in the original changenote. We should address the qualitative comments as well in |
Update original change note in line with the change here: #14876 (comment)
This is the initial commit of all previous and the current (2.15.3) version changelogs, based on the current state of the
codeql-cli-2.15.3branch.I've previewed the changes locally and all seem to render fine.