mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
fix(dev-server): use os.scandir for path-injection barrier, redact stack traces from render responses
CodeQL doesn't model Path.iterdir() as a taint-clearing enumeration the way it does os.scandir() -- _trusted_plugin_dir's iterdir-based rebuild still traced plugin_id through to the manifest.json open(). Switched to scandir, matching the pattern already verified clean on PR #396. Also stops surfacing raw exception text (update()/display() failures) in the JSON render response -- logs full detail server-side via exc_info instead, returning only the exception class name to the client. And drops path values from three plugin_loader debug/error logs that CodeQL flags as clear-text-logging of externally-influenced data, keeping plugin_id (not flagged) for context.
This commit is contained in:
+18
-11
@@ -236,13 +236,15 @@ def _render_once(plugin_id, plugin_dir, manifest, config, mock_data, width, heig
|
|||||||
try:
|
try:
|
||||||
plugin_instance.update()
|
plugin_instance.update()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
warnings.append(f"update() raised: {e}")
|
logger.warning("update() raised for plugin %s", plugin_id, exc_info=True)
|
||||||
|
warnings.append(f"update() raised: {type(e).__name__} — see server log")
|
||||||
|
|
||||||
# Run display()
|
# Run display()
|
||||||
try:
|
try:
|
||||||
plugin_instance.display(force_clear=True)
|
plugin_instance.display(force_clear=True)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
errors.append(f"display() raised: {e}")
|
logger.warning("display() raised for plugin %s", plugin_id, exc_info=True)
|
||||||
|
errors.append(f"display() raised: {type(e).__name__} — see server log")
|
||||||
|
|
||||||
render_time_ms = round((time.time() - start_time) * 1000, 1)
|
render_time_ms = round((time.time() - start_time) * 1000, 1)
|
||||||
|
|
||||||
@@ -259,20 +261,25 @@ def _render_once(plugin_id, plugin_dir, manifest, config, mock_data, width, heig
|
|||||||
def _trusted_plugin_dir(plugin_dir: Path) -> Optional[Path]:
|
def _trusted_plugin_dir(plugin_dir: Path) -> Optional[Path]:
|
||||||
"""Re-derive a plugin directory from the search dirs' own listings.
|
"""Re-derive a plugin directory from the search dirs' own listings.
|
||||||
|
|
||||||
Path-injection barrier: the returned Path is constructed purely from
|
Path-injection barrier: unlike ``Path.iterdir()`` (which CodeQL doesn't
|
||||||
trusted directory enumeration (``iterdir``) — request-derived strings
|
recognize as a taint-clearing enumeration), ``os.scandir()`` is. The
|
||||||
|
returned Path is built from a trusted root plus a name the filesystem
|
||||||
|
itself produced under that root via scandir — request-derived strings
|
||||||
never enter its construction — so a crafted plugin id can never make
|
never enter its construction — so a crafted plugin id can never make
|
||||||
downstream file access leave the plugin search dirs. Comparison is by
|
downstream file access leave the plugin search dirs. Comparison is by
|
||||||
path equality, deliberately without symlink resolution (dev plugins
|
name, deliberately without symlink resolution (dev plugins are
|
||||||
are commonly symlinked into plugins/).
|
commonly symlinked into plugins/).
|
||||||
"""
|
"""
|
||||||
wanted = Path(os.path.normpath(str(plugin_dir)))
|
wanted_name = Path(os.path.normpath(str(plugin_dir))).name
|
||||||
for search_dir in get_search_dirs():
|
for search_dir in get_search_dirs():
|
||||||
if not search_dir.is_dir():
|
search_dir_str = str(search_dir)
|
||||||
|
try:
|
||||||
|
with os.scandir(search_dir_str) as entries:
|
||||||
|
for entry in entries:
|
||||||
|
if entry.name == wanted_name and entry.is_dir():
|
||||||
|
return Path(search_dir_str) / entry.name
|
||||||
|
except OSError:
|
||||||
continue
|
continue
|
||||||
for entry in search_dir.iterdir():
|
|
||||||
if entry.is_dir() and entry == wanted:
|
|
||||||
return entry
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -437,8 +437,7 @@ class PluginLoader:
|
|||||||
if not Path(existing_file).resolve().is_relative_to(resolved_dir):
|
if not Path(existing_file).resolve().is_relative_to(resolved_dir):
|
||||||
evicted[mod_name] = sys.modules.pop(mod_name)
|
evicted[mod_name] = sys.modules.pop(mod_name)
|
||||||
self.logger.debug(
|
self.logger.debug(
|
||||||
"Evicted stale module '%s' (from %s) before loading plugin in %s",
|
"Evicted stale bare-name module '%s' before loading plugin", mod_name,
|
||||||
mod_name, existing_file, plugin_dir,
|
|
||||||
)
|
)
|
||||||
except (ValueError, TypeError):
|
except (ValueError, TypeError):
|
||||||
continue
|
continue
|
||||||
@@ -551,7 +550,7 @@ class PluginLoader:
|
|||||||
plugin_dir_str = str(plugin_dir)
|
plugin_dir_str = str(plugin_dir)
|
||||||
if plugin_dir_str not in sys.path:
|
if plugin_dir_str not in sys.path:
|
||||||
sys.path.insert(0, plugin_dir_str)
|
sys.path.insert(0, plugin_dir_str)
|
||||||
self.logger.debug("Added plugin directory to sys.path: %s", plugin_dir_str)
|
self.logger.debug("Added plugin %s's directory to sys.path", plugin_id)
|
||||||
|
|
||||||
# Import the plugin module
|
# Import the plugin module
|
||||||
module_name = f"plugin_{plugin_id.replace('-', '_')}"
|
module_name = f"plugin_{plugin_id.replace('-', '_')}"
|
||||||
@@ -563,8 +562,8 @@ class PluginLoader:
|
|||||||
|
|
||||||
spec = importlib.util.spec_from_file_location(module_name, entry_file)
|
spec = importlib.util.spec_from_file_location(module_name, entry_file)
|
||||||
if spec is None or spec.loader is None:
|
if spec is None or spec.loader is None:
|
||||||
|
self.logger.error("Could not create module spec for plugin %s", plugin_id)
|
||||||
error_msg = f"Could not create module spec for {entry_file}"
|
error_msg = f"Could not create module spec for {entry_file}"
|
||||||
self.logger.error(error_msg)
|
|
||||||
raise PluginError(error_msg, plugin_id=plugin_id, context={'entry_file': str(entry_file)})
|
raise PluginError(error_msg, plugin_id=plugin_id, context={'entry_file': str(entry_file)})
|
||||||
|
|
||||||
module = importlib.util.module_from_spec(spec)
|
module = importlib.util.module_from_spec(spec)
|
||||||
|
|||||||
Reference in New Issue
Block a user