Include VSCode Insiders extensions in vscode_extensions table #8396
Conversation
Smjert
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
I have a minor ask to better support potential expansion to this logic.
| for (const auto& confDir : confDirs) { | ||
| // Determine the version type | ||
| std::string version_type = | ||
| (confDir.second.string().find("insiders") != std::string::npos) |
There was a problem hiding this comment.
Instead of making this generic check, I would just add the information we need in the confDirs set, so we have the correct information already there.
So store a struct with 3 values in it instead of the std::pair, with the third being the version_type.
And at this point I would also make confDirs -> conf_dirs to be consistent with the variable naming scheme.
There was a problem hiding this comment.
That makes sense, think that would be clearer too.
I didn't have time to make the change during the weekend but will do in the next couple days. Thank you for the feedback.
There was a problem hiding this comment.
Ok I made a ConfDir struct that contains the uid, path and version_type and am using it inside the conf_dirs set.
Let me know if this is not what you had in mind since my C++ is not great
|
I just tested this on macOS Sonoma using the current versions of Visual Studio Code (1.92.2) and Visual Studio Code Insiders (1.93.0), with the two apps installed side-by-side (well, the app bundles can be anywhere, but the settings directories are side-by-side). Works well 👍 |
specs/vscode_extensions.table
Outdated
| Column("prerelease", INTEGER, "Pre release version"), | ||
| Column("uid", BIGINT, "The local user that owns the plugin", | ||
| additional=True), | ||
| Column("version_type", TEXT, "VSCode or VSCode Insiders") |
There was a problem hiding this comment.
I'm not sure version_type is the right name. Is this about whether the vscode itself is running an insiders/beta edition or is it specific to the extension?
Reading the code, this is switching off the path. I don't know the ecosystem here, but is server something we'd want to expose?
Would we want this to be vscode_edition? Or omit it, and let clients infer from the path, the same way this does.
There was a problem hiding this comment.
For server, we’re already exposing it for the regular VSCode version. If I’m running osquery on WSL and have VSCode on my Windows, the extensions come from the /home/.../.vscode-server/extensions path, for example.
And I agree version_type may not be the best name, as it could get confused with the extension’s version type. vscode_edition seems better. We could omit it, but I think having it as a single column adds clarity.
So maybe keep exposing server and change the column to vscode_edition?
There was a problem hiding this comment.
Yeah, I think keeping it, but naming it vscode_edition makes more sense.
There was a problem hiding this comment.
Sounds good, I kept it and changed the name to vscode_edition
Include VSCode Insiders extensions in vscode_extensions table
Background
As per this issue -> #8350 <- this PR introduces VSCode insiders extensions in the
vscode_extensionstable.Changes
This changes:
osquery/tables/applications/vscode_extensions.cpp- Add path for.vscode-server-insidersand.vscode-insiders/extensions/extensions.jsonfiles, and also adds a new column namedversion_typeto differentiate betweenvscodeorvscode_insiders.specs/vscode_extensions.tableandtests/integration/tables/vscode_extensions.cppto include the new column.