Skip to content

fix(OSQUERY-007): CU-86akhf8u2 3 review findings across 3 files #101

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.

Sign up for GitHub

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

Draft
flamingo wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

fix(OSQUERY-007): CU-86akhf8u2 3 review findings across 3 files #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e05173f
fix(OSQUERY-007): 3 review findings across 3 files
flamingo[bot] Sep 28, 2026
8f0a92e
fix(OSQUERY-007): 3 review findings across 3 files
flamingo[bot] Sep 28, 2026
313bd99
fix(OSQUERY-007): 3 review findings across 3 files
flamingo[bot] Sep 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions osquery/events/darwin/es_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,4 @@ void getProcessProperties(const es_process_t* p,
ec->cwd = getCwdPathFromPid(ec->pid);
}

void appendQuotedString(std::ostream& out, std::string s, char delim) {

@flamingo flamingo Bot Sep 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 appendQuotedString is unused and placed at exported namespace scope instead of anonymous namespace

Removed the unused, exported appendQuotedString function from osquery/events/darwin/es_utils.cpp (previously defined at file scope in namespace osquery, near the end of the file after getProcessProperties). Since the finding confirms zero callers anywhere in the org, deletion (the "remove if leftover scaffolding" option offered) is the minimal fix rather than adding an anonymous namespace around dead code; this also removes the now-unneeded <iomanip> usage site (header left in place since it's still a plausible general-purpose include, and removing it is out of scope for this finding).

🤖 Prompt for AI agents 
In osquery/events/darwin/es_utils.cpp around line 165, review and complete this code-review fix: appendQuotedString is unused and placed at exported namespace scope instead of anonymous namespace.
What the draft fix changed: Removed the unused, exported `appendQuotedString` function from `osquery/events/darwin/es_utils.cpp` (previously defined at file scope in `namespace osquery`, near the end of the file after `getProcessProperties`). Since the finding confirms zero callers anywhere in the org, deletion (the "remove if leftover scaffolding" option offered) is the minimal fix rather than adding an anonymous namespace around dead code; this also removes the now-unneeded `<iomanip>` usage site (header left in place since it's still a plausible general-purpose include, and removing it is out of scope for this finding).
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

if (s.find(delim) != std::string::npos || s.find('"') != std::string::npos) {
out << std::quoted(s) << delim;
} else {
out << s << delim;
}
}

} // namespace osquery
4 changes: 4 additions & 0 deletions osquery/tables/system/darwin/homebrew_packages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const std::set<std::string> kHomebrewPrefixes = {
"/opt/homebrew",
};

namespace {

std::vector<std::string> getHomebrewAppInfoPlistPaths(const std::string& root) {

@flamingo flamingo Bot Sep 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 File-scope helper functions in homebrew_packages.cpp are not anonymous-namespace or static

Wrapped all file-local helper functions (getHomebrewAppInfoPlistPaths, getHomebrewNameFromInfoPlistPath, getHomebrewVersionsFromInfoPlistPath, checkAutoUpdatesInRubyFile, getBooleanValueFromJsonFile, getAppNameFromJsonManifest, getAppNameFromRubyManifest, getMetadataFileForCask, getHomebrewAutoUpdate, getInstalledAppNameFromMetadata, computeVersionsForFormulas, computeVersionsForCasks, packagesFromPrefix) inside an anonymous namespace { ... } block within namespace osquery { namespace tables { ... } } in osquery/tables/system/darwin/homebrew_packages.cpp, giving them internal linkage. The exported entry point genHomebrewPackages remains outside the anonymous namespace with external linkage, unchanged, and no header declares these helpers so no external caller is affected.

🤖 Prompt for AI agents 
In osquery/tables/system/darwin/homebrew_packages.cpp around line 41, review and complete this code-review fix: File-scope helper functions in homebrew_packages.cpp are not anonymous-namespace or static.
What the draft fix changed: Wrapped all file-local helper functions (getHomebrewAppInfoPlistPaths, getHomebrewNameFromInfoPlistPath, getHomebrewVersionsFromInfoPlistPath, checkAutoUpdatesInRubyFile, getBooleanValueFromJsonFile, getAppNameFromJsonManifest, getAppNameFromRubyManifest, getMetadataFileForCask, getHomebrewAutoUpdate, getInstalledAppNameFromMetadata, computeVersionsForFormulas, computeVersionsForCasks, packagesFromPrefix) inside an anonymous `namespace { ... }` block within `namespace osquery { namespace tables { ... } }` in osquery/tables/system/darwin/homebrew_packages.cpp, giving them internal linkage. The exported entry point `genHomebrewPackages` remains outside the anonymous namespace with external linkage, unchanged, and no header declares these helpers so no external caller is affected.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

std::vector<std::string> results;
auto status = osquery::listDirectoriesInDirectory(root, results);
Expand Down Expand Up @@ -338,6 +340,8 @@ void packagesFromPrefix(QueryData& results,
computeVersionsForCasks(results, prefix, userRequested);
}

} // namespace

QueryData genHomebrewPackages(QueryContext& context) {
QueryData results;

Expand Down
9 changes: 7 additions & 2 deletions osquery/tables/system/windows/shimcache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
#include <algorithm>
#include <string>

namespace osquery {
namespace tables {

namespace {

const int kWin8 = 256;

@flamingo flamingo Bot Sep 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 shimcache.cpp declares file-scope constants and struct outside any namespace

Moved kWin8, kWin10PreCreator, kWin10Creator, kWin8Start, kWin10Start, kWin10CreatorStart, kWin8110ShimcacheDelimiter, kShimcacheControlset, and struct ShimcacheData from true global (file) scope into an anonymous namespace { ... } block nested inside namespace osquery { namespace tables { ... } }, matching the established pattern used elsewhere in the codebase. No usages of these names elsewhere in the file needed changes since they are used only within functions already defined inside osquery::tables.

🤖 Prompt for AI agents 
In osquery/tables/system/windows/shimcache.cpp around line 25, review and complete this code-review fix: shimcache.cpp declares file-scope constants and struct outside any namespace.
What the draft fix changed: Moved `kWin8`, `kWin10PreCreator`, `kWin10Creator`, `kWin8Start`, `kWin10Start`, `kWin10CreatorStart`, `kWin8110ShimcacheDelimiter`, `kShimcacheControlset`, and `struct ShimcacheData` from true global (file) scope into an anonymous `namespace { ... }` block nested inside `namespace osquery { namespace tables { ... } }`, matching the established pattern used elsewhere in the codebase. No usages of these names elsewhere in the file needed changes since they are used only within functions already defined inside `osquery::tables`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

const int kWin10PreCreator = 96;
const int kWin10Creator = 104;
Expand All @@ -44,8 +49,7 @@ struct ShimcacheData {
boost::optional<bool> execution_flag;
};

namespace osquery {
namespace tables {
} // namespace

auto parseShimcacheData(const std::string& token,
const boost::optional<bool>& execution_flag_exists) {
Expand Down Expand Up @@ -227,3 +231,4 @@ QueryData genShimcache(QueryContext& context) {

} // namespace tables
} // namespace osquery

Loading