Skip to content

fix(OSQUERY-008): CU-86akhf8u2 3 review findings across 3 files #77

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-008): CU-86akhf8u2 3 review findings across 3 files #77

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
f5837e5
fix(OSQUERY-008): 3 review findings across 3 files
flamingo[bot] Sep 21, 2026
b4d55dc
fix(OSQUERY-008): 3 review findings across 3 files
flamingo[bot] Sep 21, 2026
ed7e4e5
fix(OSQUERY-008): 3 review findings across 3 files
flamingo[bot] Sep 21, 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
3 changes: 2 additions & 1 deletion osquery/tables/system/posix/sysctl_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
namespace osquery {
namespace tables {

#define CTL_MAX_VALUE 128

@flamingo flamingo Bot Sep 21, 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.

🦩 🟠 kControlSettingsFiles / kControlSettingsDirs correctly named but CTL_MAX_VALUE macro breaks the k-prefix constant convention

Replaced #define CTL_MAX_VALUE 128 with constexpr int kCtlMaxValue = 128; inside the osquery::tables namespace in sysctl_utils.h, per OSQUERY-008 naming convention. Risk: this constant may be referenced by name CTL_MAX_VALUE in system_controls.cpp or platform-specific .cpp files (e.g. linux/darwin implementations) not visible here; if so, those call sites would fail to compile and need to be updated to kCtlMaxValue as well, which is outside this single-file fix's scope.

🤖 Prompt for AI agents 
In osquery/tables/system/posix/sysctl_utils.h around line 19, review and complete this code-review fix: kControlSettingsFiles / kControlSettingsDirs correctly named but CTL_MAX_VALUE macro breaks the k-prefix constant convention.
What the draft fix changed: Replaced `#define CTL_MAX_VALUE 128` with `constexpr int kCtlMaxValue = 128;` inside the `osquery::tables` namespace in sysctl_utils.h, per OSQUERY-008 naming convention. Risk: this constant may be referenced by name `CTL_MAX_VALUE` in system_controls.cpp or platform-specific .cpp files (e.g. linux/darwin implementations) not visible here; if so, those call sites would fail to compile and need to be updated to `kCtlMaxValue` as well, which is outside this single-file fix's scope.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer

constexpr int kCtlMaxValue = 128;

#ifndef CTL_DEBUG_MAXID
#define CTL_DEBUG_MAXID (CTL_MAXNAME * 2)
Expand All @@ -41,3 +41,4 @@ void genControlInfoFromName(const std::string& name,
const std::map<std::string, std::string>& config);
}
}

9 changes: 5 additions & 4 deletions osquery/tables/system/user_groups.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include <osquery/filesystem/filesystem.h>
#include <osquery/logger/logger.h>

#define EXPECTED_GROUPS_MAX 64

@flamingo flamingo Bot Sep 21, 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.

🦩 🟠 EXPECTED_GROUPS_MAX macro violates kPascalCase constant naming convention

Replaced #define EXPECTED_GROUPS_MAX 64 with constexpr int kExpectedGroupsMax = 64; and updated all three usages (array bound, ngroups initialization, and the comment) in getGroupsForUser to reference kExpectedGroupsMax, satisfying OSQUERY-008 kPascalCase naming for the constant.

🤖 Prompt for AI agents 
In osquery/tables/system/user_groups.h around line 23, review and complete this code-review fix: EXPECTED_GROUPS_MAX macro violates kPascalCase constant naming convention.
What the draft fix changed: Replaced `#define EXPECTED_GROUPS_MAX 64` with `constexpr int kExpectedGroupsMax = 64;` and updated all three usages (array bound, `ngroups` initialization, and the comment) in `getGroupsForUser` to reference `kExpectedGroupsMax`, satisfying OSQUERY-008 kPascalCase naming for the constant.
Verify the change is correct and complete; do not refactor unrelated code.

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

constexpr int kExpectedGroupsMax = 64;

#ifdef __APPLE__
// This symbol is exported from libSystem.B and has been since 10.6.
Expand Down Expand Up @@ -65,14 +65,14 @@ static void getGroupsForUser(QueryData& results,
}
delete[] groups;
#else
gid_type groups_buf[EXPECTED_GROUPS_MAX];
gid_type groups_buf[kExpectedGroupsMax];
gid_type* groups = groups_buf;
int ngroups = EXPECTED_GROUPS_MAX;
int ngroups = kExpectedGroupsMax;

// GLIBC version before 2.3.3 may have a buffer overrun:
// http://man7.org/linux/man-pages/man3/getgrouplist.3.html
if (getgrouplist(user.name, user.gid, groups, &ngroups) < 0) {
// EXPECTED_GROUPS_MAX was probably not large enough.
// kExpectedGroupsMax was probably not large enough.
// Try a larger size buffer.
groups = new gid_type[ngroups];
if (groups == nullptr) {
Expand All @@ -96,3 +96,4 @@ static void getGroupsForUser(QueryData& results,
}
} // namespace tables
} // namespace osquery

11 changes: 6 additions & 5 deletions osquery/utils/info/tool_type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,24 @@ namespace osquery {
namespace {

/// Current tool type.

@flamingo flamingo Bot Sep 21, 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 constant kToolType does not follow the constant style but this is a mutable global, not a constant

Renamed the mutable global variable kToolType to gToolType in osquery/utils/info/tool_type.cpp, updating its declaration and all usages within setToolType(), getToolType(), isDaemon(), and isShell() to remove the misleading 'k' constant-prefix convention from a variable that is actually mutated.

🤖 Prompt for AI agents 
In osquery/utils/info/tool_type.cpp around line 16, review and complete this code-review fix: File-scope constant kToolType does not follow the constant style but this is a mutable global, not a constant.
What the draft fix changed: Renamed the mutable global variable `kToolType` to `gToolType` in `osquery/utils/info/tool_type.cpp`, updating its declaration and all usages within `setToolType()`, `getToolType()`, `isDaemon()`, and `isShell()` to remove the misleading 'k' constant-prefix convention from a variable that is actually mutated.
Verify the change is correct and complete; do not refactor unrelated code.

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

ToolType kToolType{ToolType::UNKNOWN};
ToolType gToolType{ToolType::UNKNOWN};

} // namespace

void setToolType(ToolType tool) {
kToolType = tool;
gToolType = tool;
}

ToolType getToolType() {
return kToolType;
return gToolType;
}

bool isDaemon() {
return kToolType == ToolType::DAEMON;
return gToolType == ToolType::DAEMON;
}

bool isShell() {
return kToolType == ToolType::SHELL;
return gToolType == ToolType::SHELL;
}
} // namespace osquery

Loading