mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-09 08:41:14 +00:00
fix: avoid substring denies in audit path keywords (#16243)
## Summary Fixes #15944. The plugin audit deny-list matched `secret`, `cert`, and `conf` as substrings of every path component. This blocked valid imports during plugin capability execution, for example `numpy/__config__.py`, because `conf` appeared inside the module filename. This PR changes deny keyword matching to use whole path components instead of substring matches. It keeps the intended protections for sensitive locations and config files, while allowing dependency and stdlib modules whose names merely contain those strings. ## Changes - Match denied path keywords as whole components instead of substrings. - Keep denying sensitive directory names such as: - `secret` - `secrets` - `cert` - `certs` - `certificate` - `certificates` - `conf` - `config` - Keep denying config files by extension: - `.conf` - `.ini` - Allow legitimate Python module/package paths such as: - `numpy/__config__.py` - `numpy/_core/_ufunc_config.py` - `configparser.py` - `sysconfig.py` - `logging/config.py` - `certifi/cacert.pem` - Include the denied target and reason in `PermissionError` messages when the audit hook blocks an operation. - Remove an unused `<memory>` include from `PluginAuditManager.hpp`. ## Why The previous substring matching caused false positives for common dependency and standard-library paths. It also made failures hard to diagnose because the Python exception did not include the refused path. The new behavior is narrower: it blocks sensitive path components and config file extensions without treating unrelated names like `__config__.py`, `configparser.py`, `Conference`, or `Concert` as secrets. ## Testing - Added/updated unit coverage in `tests/slic3rutils/test_plugin_audit.cpp` for: - whole-component keyword matches - `.conf` / `.ini` blocking - case-insensitive matching - false-positive paths from #15944 Plugin used for testing: [orca_audit_numpy_config_repro.py](https://github.com/user-attachments/files/33143848/orca_audit_numpy_config_repro.py)
This commit is contained in:
@@ -351,8 +351,20 @@ std::vector<std::string> PluginAuditManager::default_denied_path_keywords()
|
||||
// must never be able to reach a secret, a certificate, or a configuration file just because
|
||||
// it happens to live inside an otherwise-allowed root (e.g. the bundled TLS client cert at
|
||||
// resources_dir()/cert/..., which would become reachable the moment resources_dir() is
|
||||
// granted as a read-only allowed root).
|
||||
return {"secret", "cert", "conf"};
|
||||
// granted as a read-only allowed root). Match as whole path components, not substrings, so
|
||||
// imports such as numpy/__config__.py and stdlib configparser.py remain usable.
|
||||
return {"secret", "secrets", "cert", "certs", "certificate", "certificates", "conf", "config"};
|
||||
}
|
||||
|
||||
static bool has_denied_config_extension(std::string name)
|
||||
{
|
||||
const size_t stream_pos = name.find(':');
|
||||
if (stream_pos != std::string::npos)
|
||||
name.erase(stream_pos);
|
||||
|
||||
const boost::filesystem::path path(name);
|
||||
const std::string extension = path.extension().string();
|
||||
return extension == ".conf" || extension == ".ini";
|
||||
}
|
||||
|
||||
bool PluginAuditManager::is_denied_path_keyword(const boost::filesystem::path& candidate) const
|
||||
@@ -377,7 +389,7 @@ bool PluginAuditManager::is_denied_path_keyword(const boost::filesystem::path& c
|
||||
continue;
|
||||
std::transform(name.begin(), name.end(), name.begin(), [](unsigned char c) { return std::tolower(c); });
|
||||
for (const auto& keyword : m_denied_path_keywords) {
|
||||
if (name.find(keyword) != std::string::npos)
|
||||
if (name == keyword || (keyword == "conf" && has_denied_config_extension(name)))
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -810,7 +822,8 @@ bool persist_permission(const std::string& plugin_key,
|
||||
|
||||
int report_denied(PluginAuditManager& mgr,
|
||||
const std::string& event_name,
|
||||
const AuditDecision& decision)
|
||||
const AuditDecision& decision,
|
||||
const std::string& target = {})
|
||||
{
|
||||
AuditViolation violation;
|
||||
violation.plugin_key = mgr.current_plugin();
|
||||
@@ -818,7 +831,13 @@ int report_denied(PluginAuditManager& mgr,
|
||||
violation.reason = decision.reason;
|
||||
mgr.report_violation(violation);
|
||||
|
||||
PyErr_SetString(PyExc_PermissionError, "Plugin attempted an audited operation without permission");
|
||||
std::string message = "Plugin attempted audited operation \"" + event_name + "\" without permission";
|
||||
if (!decision.reason.empty())
|
||||
message += ": " + decision.reason;
|
||||
if (!target.empty())
|
||||
message += ": " + target;
|
||||
|
||||
PyErr_SetString(PyExc_PermissionError, message.c_str());
|
||||
return -1;
|
||||
}
|
||||
|
||||
@@ -987,7 +1006,7 @@ int PluginAuditManager::audit_hook(const char* event, PyObject* args, void* user
|
||||
if (fs_category) {
|
||||
for (const auto& target : targets) {
|
||||
if (mgr->is_denied_path(boost::filesystem::path(target)))
|
||||
return PluginAuditDetail::report_denied(*mgr, event_name, {false, "denied path"});
|
||||
return PluginAuditDetail::report_denied(*mgr, event_name, {false, "denied path"}, target);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
// Via pybind11 so this file requests the same python3xx.lib as everything else.
|
||||
#include <boost/filesystem/path.hpp>
|
||||
#include <pybind11/conduit/wrap_include_python_h.h>
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <string>
|
||||
#include <unordered_map>
|
||||
@@ -104,22 +103,20 @@ public:
|
||||
bool is_denied_filename(const boost::filesystem::path& candidate) const;
|
||||
|
||||
// --- denied-path-keyword registry ---
|
||||
// Keywords that categorically deny a path if ANY of its components (directory or file
|
||||
// name), not just the base name, contains one case-insensitively -- e.g. a "secrets"
|
||||
// subfolder, a "certificates" folder, or a "conf"/"config" file anywhere the plugin can
|
||||
// otherwise reach, including inside an allowed root. This is intentionally broader and
|
||||
// fuzzier than the exact-name is_denied_filename registry: it exists to categorically rule
|
||||
// out whole classes of sensitive paths (secrets, certificates, config) rather than name
|
||||
// specific known files, at the cost of over-blocking an unrelated name that happens to
|
||||
// contain the keyword -- the fail-safe direction, same rationale as is_denied_filename.
|
||||
// Keywords that categorically deny a path if ANY component matches one case-insensitively --
|
||||
// e.g. a "secrets" subfolder, a "certificates" folder, a "conf"/"config" directory, or a
|
||||
// .conf/.ini file anywhere the plugin can otherwise reach, including inside an allowed root.
|
||||
// This is broader than the exact-name is_denied_filename registry, but it is not a substring
|
||||
// match: importable modules such as numpy/__config__.py, configparser.py, sysconfig.py, or
|
||||
// user folders such as "Conference" and "Concert" are unrelated names and must stay promptable.
|
||||
void add_denied_path_keyword(const std::string& keyword);
|
||||
|
||||
// The list install_hook() seeds into the keyword registry. Exposed so tests seed the exact
|
||||
// same set without a live interpreter.
|
||||
static std::vector<std::string> default_denied_path_keywords();
|
||||
|
||||
// True when any component of candidate's (canonicalized) path contains a registered
|
||||
// keyword, case-insensitively.
|
||||
// True when any component of candidate's (canonicalized) path matches a registered keyword,
|
||||
// case-insensitively. A registered "conf" keyword also blocks .conf/.ini file components.
|
||||
bool is_denied_path_keyword(const boost::filesystem::path& candidate) const;
|
||||
|
||||
// is_denied_filename(candidate) || is_denied_path_keyword(candidate). Convenience for
|
||||
|
||||
@@ -222,18 +222,19 @@ TEST_CASE("Plugin audit denies secret/certificate/config-like paths by keyword",
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/resources/certificates/ca.pem")));
|
||||
}
|
||||
|
||||
SECTION("a 'conf'/'config' directory or file component is denied")
|
||||
SECTION("a 'conf'/'config' directory or config file component is denied")
|
||||
{
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/conf/settings.json")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/config/settings.json")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/plugin.conf")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/plugin.ini")));
|
||||
}
|
||||
|
||||
SECTION("matching is case-insensitive")
|
||||
{
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/SECRETS/token.txt")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/resources/CertBundle/ca.pem")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/CONFIG.JSON")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/resources/Certificates/ca.pem")));
|
||||
CHECK(mgr.is_denied_path_keyword(fs::path("/plugin/PLUGIN.CONF")));
|
||||
}
|
||||
|
||||
SECTION("matching is not limited to the base name -- any ancestor component counts")
|
||||
@@ -245,6 +246,14 @@ TEST_CASE("Plugin audit denies secret/certificate/config-like paths by keyword",
|
||||
{
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/plugin/output/model.gcode")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/plugin/storage/state.json")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/packages/cp312/numpy/__config__.py")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/packages/cp312/numpy/_core/_ufunc_config.py")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/Lib/configparser.py")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/Lib/sysconfig.py")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/Lib/logging/config.py")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/python/packages/cp312/certifi/cacert.pem")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/users/Conference/output.txt")));
|
||||
CHECK_FALSE(mgr.is_denied_path_keyword(fs::path("/users/Concert/output.txt")));
|
||||
}
|
||||
|
||||
SECTION("an empty path is not denied")
|
||||
|
||||
Reference in New Issue
Block a user