-
Notifications
You must be signed in to change notification settings - Fork 202
Extension should notify users when their QL libraries are out of date #495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
As a security researcher playing with a one-off query, I'd probably be fine with always using the latest official QL libraries. As a QL query author developing a query to be used by the rest of my team or company, I really need my libraries to match the ones that will actually be used by my teammates and CI systems. As a QL library author, I need my libraries to match the ones I expect consumers of my library to use. And in any of these scenarios, if the latest official library contains a breaking change, I don't want the extension to automatically update the libraries and break me, I don't want the extension to prompt me "Do you want to upgrade to enable these cool IDE features? Warning: this may break your code!", and I still want to use the cool IDE features if I can. It appears that the main reason we would need to have the user update the libraries is because we've chosen to distribute the IDE queries as part of the standard library. However, it looks like the IDE queries we have today depend on a relatively small surface area of the standard libraries. The In the future, a combination of non-destructive database upgrades and true query packages will probably make this easier, but I think we can still make the experience better today without having to ask users to upgrade the libraries their code depends on because their IDE wants to depend on a different library version. Actual work required:
|
That's a great idea. It is kind of odd to have extension features be dependent on the sources in your workspace. Your proposal is not going to completely get around that, but at least it puts extension-specific bits in the extension repo. This will make our logic for generating qlpacks slightly more complex, but still doable. One question: Are version checks already implemented? If so, I think the extension wouldn't need to check versions, it just attempts to run the query and if the compiler fails due to bad version constraints, then we report an understandable message back to the user. |
So, there is a small concern I have with your proposal. I'm not sure how much to make of it since this architectural decision was made before I started. The way that contextual queries are implemented now, the extension doesn't need to know anything about which languages, libraries, or extractors are available. The extension just knows to run query X, with some bound externals and using the default query pack. So, as long as the standard libraries for a given language implement the query correctly, the contextual queries can run properly. The extension doesn't need to change to fully support Ruby (for example). I can see the advantages for separating responsibilities in this manner, but I'm not arguing too strongly for it since I also see the advantage of leaving IDE-things in the IDE. Though, I've had a similar conversation with @p0 and @jcreedcmu in the past., so they may have something different to say. |
Side note: We should be imagining a world in which the starter repo is not necessary in order to write or use a query. At some point, the process of writing a new query will be basically:
(or the IDE equivalent, of course). The starter repo may still exist, and may still be a useful way to play around with and modify existing queries, but we shouldn't assume that the user has a checkout of the starter repo at all. If the user is writing a query, there will at least be a copy of the standard library installed at some point via the package manager, so we'd still have access to that if we needed it. If the user is merely fixing alerts from an existing query, or just exploring a source code base using CodeQL-assisted contextual queries, there may not be a standard library on the user's machine at all. I don't know how much we care about this last scenario, though. |
I might be overthinking this. If we include the contextual queries themselves in the standard library, what happens at each of the following events? Changes to the standard library without affecting the contextual query itself Changes to the standard library that affect the results of the contextual query, without changing the contextual query itself Changes to the contextual query without changing the standard library itself CodeQL for VS Code implements support for a new contextual query Here's a thought: What if, whenever we start to support a new contextual query, we write the implementation for each language that we care about, and include that implementation within the extension so it's always available, and have it depend on the lowest version of the standard library that it can reasonably support. We also commit that same implementation to the repo for the relevant standard library, or, in the case of an external language, submit a PR to that language's repo. If the user's standard library already has an implementation of the query, we use that. Otherwise, we fall back to the one bundled with the extension if one exists. If we don't have one of those, either, we fail gracefully. Users get to use the new query immediately, implementers of other languages can add support at their leisure, although it will force their users to upgrade to a new standard library to take advantage of it. And if a language implementer does want to allow existing users to use the query, they can submit a PR to the VS Code extension to add the bundled copy. If we ever get to the point where this happens so often that we want a more sophisticated way of automatically discovering a separate query pack containing the implementations of contextual queries for a particular language without bundling them, I think we'll be clearly into "nice problem to have" territory, and can figure it out then. |
Background
Most users consume QL through the vscode-codeql-starter workspace. Some cli and ide features not work unless you have up to date QL libraries. Most users aren't aware of this and there is no mechanism to notify users that updates need to happen. The only situation where users are notified is when they add a new database built from a later version of the QL libraries. However, this is not sufficient.
Proposed solution
The extension should periodically check to see if there are new QL libraries available and warn the user if there are. Optionally, the extension could try to install the changes automatically. Otherwise, clear manual instructions should be given.
Caveats and questions
It might look like this:
The text was updated successfully, but these errors were encountered: