Skip to content
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

New Table: Windows Update History #7407

Merged

Conversation

aleksmaus
Copy link
Contributor

This change adds the new table windows_update_history as requested in the issue: #7405

Screenshot:
Screen Shot 2021-12-06 at 2 43 17 PM

Opening for a feedback.

Fixes: #7405

@aleksmaus aleksmaus requested review from a team as code owners December 6, 2021 21:29
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 6, 2021

CLA Signed

The committers are authorized under a signed CLA.

@aleksmaus aleksmaus closed this Dec 8, 2021
@aleksmaus aleksmaus reopened this Dec 8, 2021
@aleksmaus
Copy link
Contributor Author

/easycla

Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a version of this in launcher and I'm excited to see this get added to osquery core. I don't feel like I can review the windows c++ code, but 👍 to the intent

Copy link
Member

@alessandrogario alessandrogario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @aleksmaus

Thanks for the PR! I have left some comments on the code

osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
osquery/tables/system/windows/windows_update_history.cpp Outdated Show resolved Hide resolved
@aleksmaus
Copy link
Contributor Author

Made the implementation consistent with the established guidelines. This addresses all the latest code review feedback.

@mike-myers-tob mike-myers-tob added this to the 5.4.0 milestone Apr 27, 2022
@mike-myers-tob
Copy link
Member

I just tried the table on a Windows 11 VM with a lot of updates installed, and it worked great. Tests ran, pass. I reviewed the code and didn't find anything to change about it. LGTM

@Smjert
Copy link
Member

Smjert commented Apr 28, 2022

@aleksmaus please rebase this on latest master, since there's an issue with the ReadTheDocs check that is solved there.

@ukmoredec
Copy link

gentle bump to encourage getting this merged! Can help if requested.

@mike-myers-tob
Copy link
Member

@alessandrogario I think the author addressed your requested changes from before

@aleksmaus
Copy link
Contributor Author

@alessandrogario could you respond please?

@mike-myers-tob mike-myers-tob dismissed alessandrogario’s stale review June 4, 2022 05:14

Changes appear to have been addressed

@mike-myers-tob
Copy link
Member

@zwass I just checked and this still LGTM if you want to approve/merge

Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferring to @mike-myers-tob

@directionless directionless merged commit 64fdb3c into osquery:master Jul 6, 2022
@aleksmaus
Copy link
Contributor Author

Thank you!

@mike-myers-tob
Copy link
Member

Deferring to @mike-myers-tob

BTW Seph in my last pass I did look for any use of an API that allocated memory that needed to be freed by the caller. Didn't see that problem here. I meant to add that in a comment.

@bobOnGitHub
Copy link

(Windows 10, Office 2010 )

I was trying to check if some updates were installed.

I tried 3 different PS commands which all gave different results but none of them returned the updates in question.

The update installers all reported as already installed when run.

This table windows_update_history doesn't include them either.

They are office 2010 updates installed by running a MS download eg. mso2010-kb4484454-fullfile-x64-glb.exe.

Looks like updates installed this way aren't reported by any of the usual tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add virtual table for non-QFE updates (Windows only)
8 participants