From 0eecbd9458e877f980b1f5908f2abfa9df37f2dc Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:20:51 +0000 Subject: [PATCH 1/4] fix(DUP-001): 9 review findings across 4 files --- osquery/tables/applications/posix/docker.cpp | 177 ++++++++++--------- 1 file changed, 92 insertions(+), 85 deletions(-) diff --git a/osquery/tables/applications/posix/docker.cpp b/osquery/tables/applications/posix/docker.cpp index 64bd85f3041..7f0c0819733 100644 --- a/osquery/tables/applications/posix/docker.cpp +++ b/osquery/tables/applications/posix/docker.cpp @@ -525,9 +525,26 @@ QueryData genContainerLabels(QueryContext& context) { } /** - * @brief Entry point for docker_container_mounts table. + * @brief Utility method shared by docker_container_mounts and + * docker_container_ports tables to iterate over containers and extract a + * child array of details into rows via the provided callback. + * + * @param context Query context. + * @param child_path Path of the child array node to iterate (e.g. "Mounts", + * "Ports"). + * @param error_label Label used in the VLOG error message on failure. + * @param row_builder Callback invoked with the container node, the set of + * constrained ids, and the child node to populate a Row which is appended to + * results. */ -QueryData genContainerMounts(QueryContext& context) { +QueryData getContainerChildRows( + QueryContext& context, + const std::string& child_path, + const std::string& error_label, + const std::function&, + const pt::ptree&, + Row&)>& row_builder) { QueryData results; std::set ids; pt::ptree containers; @@ -539,9 +556,32 @@ QueryData genContainerMounts(QueryContext& context) { for (const auto& entry : containers) { const pt::ptree& container = entry.second; try { - for (const auto& node : container.get_child("Mounts")) { - const pt::ptree& mount = node.second; + for (const auto& node : container.get_child(child_path)) { + const pt::ptree& child = node.second; Row r; + row_builder(container, ids, child, r); + results.push_back(r); + } + } catch (const pt::ptree_error& e) { + VLOG(1) << "Error getting docker " << error_label << " " << e.what(); + } + } + + return results; +} + +/** + * @brief Entry point for docker_container_mounts table. + */ +QueryData genContainerMounts(QueryContext& context) { + return getContainerChildRows( + context, + "Mounts", + "container mounts", + [](const pt::ptree& container, + const std::set& ids, + const pt::ptree& mount, + Row& r) { r["id"] = getValue(container, ids, "Id"); r["type"] = mount.get("Type", ""); r["name"] = mount.get("Name", ""); @@ -551,14 +591,7 @@ QueryData genContainerMounts(QueryContext& context) { r["mode"] = mount.get("Mode", ""); r["rw"] = (mount.get("RW", false) ? INTEGER(1) : INTEGER(0)); r["propagation"] = mount.get("Propagation", ""); - results.push_back(r); - } - } catch (const pt::ptree_error& e) { - VLOG(1) << "Error getting docker container mounts " << e.what(); - } - } - - return results; + }); } /** @@ -605,33 +638,20 @@ QueryData genContainerNetworks(QueryContext& context) { * @brief Entry point for docker_container_ports table. */ QueryData genContainerPorts(QueryContext& context) { - QueryData results; - std::set ids; - pt::ptree containers; - Status s = getContainers(context, ids, containers); - if (!s.ok()) { - return results; - } - - for (const auto& entry : containers) { - const pt::ptree& container = entry.second; - try { - for (const auto& node : container.get_child("Ports")) { - const pt::ptree& details = node.second; - Row r; + return getContainerChildRows( + context, + "Ports", + "container ports", + [](const pt::ptree& container, + const std::set& ids, + const pt::ptree& details, + Row& r) { r["id"] = getValue(container, ids, "Id"); r["type"] = details.get("Type", ""); r["port"] = INTEGER(details.get("PrivatePort", 0)); r["host_ip"] = details.get("IP", ""); r["host_port"] = INTEGER(details.get("PublicPort", 0)); - results.push_back(r); - } - } catch (const pt::ptree_error& e) { - VLOG(1) << "Error getting docker container ports " << e.what(); - } - } - - return results; + }); } /** @@ -1080,52 +1100,6 @@ void getImageLayers(const std::string& image_id, QueryData& results) { } } -/** - * @brief Calls layer extractor for all images for docker_image_layers table - */ -void getImageLayersAll(QueryData& results) { - pt::ptree tree; - Status s = dockerApi("/images/json", tree); - if (!s.ok()) { - VLOG(1) << "Error getting docker images: " << s.what(); - return; - } - for (const auto& entry : tree) { - try { - const pt::ptree& node = entry.second; - std::string id = node.get("Id", ""); - if (boost::starts_with(id, "sha256:")) { - id.erase(0, 7); - } - getImageLayers(id, results); - } catch (const pt::ptree_error& e) { - VLOG(1) << "Error getting docker image details: " << e.what(); - } - } -} - -/** - * @brief Entry point for docker_image_layers table. - */ -QueryData genImageLayers(QueryContext& context) { - QueryData results; - pt::ptree tree; - std::vector layers; - - if (context.constraints["id"].exists( - EQUALS)) { // get layers for specific image - for (const auto& id : context.constraints["id"].getAll(EQUALS)) { - if (!checkConstraintValue(id)) { - continue; - } - getImageLayers(id, results); - } - } else { // get layers for all images - getImageLayersAll(results); - } - return results; -} - /** * @brief Image history extractor for docker_image_history table */ @@ -1163,9 +1137,19 @@ void getImageHistory(const std::string& image_id, QueryData& results) { } /** - * @brief Calls history for all images for docker_image_history table + * @brief Utility method shared by docker_image_layers and + * docker_image_history tables to invoke a per-image extractor function for + * every image reported by the docker API. + * + * @param results Results collection to populate. + * @param error_label Label used in the VLOG error message on failure. + * @param extractor Per-image extraction function (getImageLayers or + * getImageHistory). */ -void getImageHistoryAll(QueryData& results) { +void getImageDetailsAll( + QueryData& results, + const std::string& error_label, + const std::function& extractor) { pt::ptree tree; Status s = dockerApi("/images/json", tree); if (!s.ok()) { @@ -1179,13 +1163,35 @@ void getImageHistoryAll(QueryData& results) { if (boost::starts_with(id, "sha256:")) { id.erase(0, 7); } - getImageHistory(id, results); + extractor(id, results); } catch (const pt::ptree_error& e) { - VLOG(1) << "Error getting docker image history: " << e.what(); + VLOG(1) << "Error getting docker " << error_label << ": " << e.what(); } } } +/** + * @brief Entry point for docker_image_layers table. + */ +QueryData genImageLayers(QueryContext& context) { + QueryData results; + pt::ptree tree; + std::vector layers; + + if (context.constraints["id"].exists( + EQUALS)) { // get layers for specific image + for (const auto& id : context.constraints["id"].getAll(EQUALS)) { + if (!checkConstraintValue(id)) { + continue; + } + getImageLayers(id, results); + } + } else { // get layers for all images + getImageDetailsAll(results, "image details", getImageLayers); + } + return results; +} + /** * @brief Entry point for docker_image_history table. */ @@ -1199,7 +1205,7 @@ QueryData genImageHistory(QueryContext& context) { getImageHistory(id, results); } } else { - getImageHistoryAll(results); + getImageDetailsAll(results, "image history", getImageHistory); } return results; } @@ -1258,3 +1264,4 @@ QueryData genImageLabels(QueryContext& context) { } } // namespace tables } // namespace osquery + From a05cfd6b7418a5c74275cb2426289b47d75032e6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:20:52 +0000 Subject: [PATCH 2/4] fix(DUP-001): 9 review findings across 4 files --- osquery/filesystem/filesystem.cpp | 51 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/osquery/filesystem/filesystem.cpp b/osquery/filesystem/filesystem.cpp index 882ba73fe31..7fa582e644c 100644 --- a/osquery/filesystem/filesystem.cpp +++ b/osquery/filesystem/filesystem.cpp @@ -68,6 +68,26 @@ Status checkFileReadLimit(std::size_t file_size, return Status::success(); } + +Status checkPathAccess(const fs::path& path, + bool effective, + int open_mode, + int access_mode, + const char* denied_message) { + auto path_exists = pathExists(path); + if (!path_exists.ok()) { + return path_exists; + } + + if (effective) { + PlatformFile fd(path, PF_OPEN_EXISTING | open_mode); + return Status(fd.isValid() ? 0 : 1); + } else if (platformAccess(path.string(), access_mode) == 0) { + return Status::success(); + } + + return Status(1, std::string(denied_message) + path.string()); +} } // namespace Status writeTextFile(const fs::path& path, @@ -242,35 +262,13 @@ Status readFile(const fs::path& path, std::string& content, bool shouldLog) { } Status isWritable(const fs::path& path, bool effective) { - auto path_exists = pathExists(path); - if (!path_exists.ok()) { - return path_exists; - } - - if (effective) { - PlatformFile fd(path, PF_OPEN_EXISTING | PF_WRITE); - return Status(fd.isValid() ? 0 : 1); - } else if (platformAccess(path.string(), W_OK) == 0) { - return Status::success(); - } - - return Status(1, "Path is not writable: " + path.string()); + return checkPathAccess( + path, effective, PF_WRITE, W_OK, "Path is not writable: "); } Status isReadable(const fs::path& path, bool effective) { - auto path_exists = pathExists(path); - if (!path_exists.ok()) { - return path_exists; - } - - if (effective) { - PlatformFile fd(path, PF_OPEN_EXISTING | PF_READ); - return Status(fd.isValid() ? 0 : 1); - } else if (platformAccess(path.string(), R_OK) == 0) { - return Status::success(); - } - - return Status(1, "Path is not readable: " + path.string()); + return checkPathAccess( + path, effective, PF_READ, R_OK, "Path is not readable: "); } Status pathExists(const fs::path& path) { @@ -665,3 +663,4 @@ std::string lsperms(int mode) { return bits; } } // namespace osquery + From ed21cd90f350663b302175e121fe81a10a1c9d82 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:20:53 +0000 Subject: [PATCH 3/4] fix(DUP-001): 9 review findings across 4 files --- osquery/tables/utility/file.cpp | 50 ++++++++++++--------------------- 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/osquery/tables/utility/file.cpp b/osquery/tables/utility/file.cpp index f11f58b5849..1c3e6629ece 100644 --- a/osquery/tables/utility/file.cpp +++ b/osquery/tables/utility/file.cpp @@ -207,22 +207,24 @@ const std::map kTypeNames{ }; #endif -std::set getPathsFromConstraints(const QueryContext& context) { - auto constraint_it = context.constraints.find("path"); +std::set getPathsOrDirsFromConstraints( + const QueryContext& context, + const std::string& column_name, + int glob_flags) { + auto constraint_it = context.constraints.find(column_name); if (constraint_it == context.constraints.end()) { return {}; } - auto paths = constraint_it->second.getAll(EQUALS); + auto results = constraint_it->second.getAll(EQUALS); context.expandConstraints( - "path", + column_name, LIKE, - paths, + results, ([&](const std::string& pattern, std::set& out) { std::vector patterns; - auto status = - resolveFilePattern(pattern, patterns, GLOB_ALL | GLOB_NO_CANON); + auto status = resolveFilePattern(pattern, patterns, glob_flags); if (status.ok()) { for (const auto& resolved : patterns) { out.insert(resolved); @@ -231,34 +233,17 @@ std::set getPathsFromConstraints(const QueryContext& context) { return status; })); - return paths; + return results; } -std::set getDirsFromConstraints(const QueryContext& context) { - auto constraint_it = context.constraints.find("directory"); - - if (constraint_it == context.constraints.end()) { - return {}; - } - - auto directories = constraint_it->second.getAll(EQUALS); - context.expandConstraints( - "directory", - LIKE, - directories, - ([&](const std::string& pattern, std::set& out) { - std::vector patterns; - auto status = - resolveFilePattern(pattern, patterns, GLOB_FOLDERS | GLOB_NO_CANON); - if (status.ok()) { - for (const auto& resolved : patterns) { - out.insert(resolved); - } - } - return status; - })); +std::set getPathsFromConstraints(const QueryContext& context) { + return getPathsOrDirsFromConstraints( + context, "path", GLOB_ALL | GLOB_NO_CANON); +} - return directories; +std::set getDirsFromConstraints(const QueryContext& context) { + return getPathsOrDirsFromConstraints( + context, "directory", GLOB_FOLDERS | GLOB_NO_CANON); } } // namespace @@ -497,3 +482,4 @@ QueryData genFile(QueryContext& context) { } } // namespace tables } // namespace osquery + From e70c1d2641b19dfe5174fcb738279652e93373fa Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:20:54 +0000 Subject: [PATCH 4/4] fix(DUP-001): 9 review findings across 4 files --- plugins/database/rocksdb.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/database/rocksdb.cpp b/plugins/database/rocksdb.cpp index 1f5419cd69d..43ec98ecc00 100644 --- a/plugins/database/rocksdb.cpp +++ b/plugins/database/rocksdb.cpp @@ -331,6 +331,7 @@ Status RocksDBDatabasePlugin::get(const std::string& domain, } return s; } + Status RocksDBDatabasePlugin::put(const std::string& domain, const std::string& key, const std::string& value) { @@ -381,7 +382,7 @@ Status RocksDBDatabasePlugin::putBatch(const std::string& domain, Status RocksDBDatabasePlugin::put(const std::string& domain, const std::string& key, int value) { - return putBatch(domain, {std::make_pair(key, std::to_string(value))}); + return put(domain, key, std::to_string(value)); } Status RocksDBDatabasePlugin::remove(const std::string& domain,