fix(plugins): install refuses to ship an unreadable plugin tree (#111804)
A clone can land unreadable (Windows ACL inheritance -> WinError 5, a mode-000 file). Discovery now skips such a dir instead of aborting (#112293), but the install that produced it still exited 0, so the user got a plugin that silently never loads. After the clone and before anything moves into place, walk the staged tree and open every file / list every dir. On failure repair u+rX where the OS honours mode bits; if still unreadable raise PluginOperationError naming the file and the fix (icacls / chmod). The staging dir is cleaned up, nothing is installed, exit is non-zero. Fixes #111804 (its discovery half landed in #112293).
This commit is contained in:
@@ -611,6 +611,43 @@ def _read_manifest_for_install(plugin_dir: Path) -> dict:
|
||||
return manifest
|
||||
|
||||
|
||||
def _probe_readable(path: Path) -> None:
|
||||
"""Raise ``OSError`` unless *path* can actually be listed (dir) or opened for reading (file)."""
|
||||
if path.is_dir():
|
||||
os.listdir(path)
|
||||
else:
|
||||
with open(path, "rb"):
|
||||
pass
|
||||
|
||||
|
||||
def _ensure_tree_readable(root: Path, plugins_dir: Path) -> None:
|
||||
"""Refuse to ship a tree Hermes cannot read back. A clone can land unreadable (Windows ACL
|
||||
inheritance -> WinError 5, a mode-000 file) and discovery would then skip the plugin forever
|
||||
(#111804); repair ``u+rX`` where the OS supports it, otherwise fail before anything moves."""
|
||||
paths = [root]
|
||||
for dirpath, dirnames, filenames in os.walk(root):
|
||||
paths.extend(Path(dirpath) / name for name in (*dirnames, *filenames))
|
||||
for path in paths:
|
||||
try:
|
||||
_probe_readable(path)
|
||||
continue
|
||||
except OSError:
|
||||
if os.name != "nt": # chmod only toggles the read-only bit on Windows; ACLs need icacls
|
||||
try:
|
||||
os.chmod(path, os.stat(path).st_mode | (0o500 if path.is_dir() else 0o400))
|
||||
except OSError:
|
||||
pass
|
||||
try:
|
||||
_probe_readable(path)
|
||||
except OSError as exc:
|
||||
fix = (f'icacls "{plugins_dir}" /grant:r "%USERNAME%":(OI)(CI)F /T' if os.name == "nt"
|
||||
else f"chmod -R u+rX {plugins_dir}")
|
||||
raise PluginOperationError(
|
||||
f"Installed file {path.relative_to(root)} is not readable ({exc.strerror or exc}); "
|
||||
f"nothing was installed. Fix permissions on {plugins_dir} (e.g. `{fix}`) and retry."
|
||||
) from exc
|
||||
|
||||
|
||||
def _swap_in_plugin(tmp_target: Path, target: Path, backup: Path, old_metadata: dict, new_metadata: dict) -> None:
|
||||
"""Move the validated clone into place and persist metadata; on any failure restore the
|
||||
previous tree (if one was replaced) and the previous metadata sidecar, then re-raise."""
|
||||
@@ -661,6 +698,7 @@ def _install_plugin_core(
|
||||
tmp_clone = Path(tmp) / "plugin"
|
||||
installed_revision = _clone_plugin_repo(tmp_clone, git_url, requested_revision)
|
||||
tmp_target = _resolve_subdir_within(tmp_clone, subdir) if subdir else tmp_clone
|
||||
_ensure_tree_readable(tmp_target, plugins_dir)
|
||||
manifest = _read_manifest_for_install(tmp_target)
|
||||
plugin_name = manifest.get("name") or (
|
||||
subdir.rstrip("/").rsplit("/", 1)[-1] if subdir else _repo_name_from_url(git_url))
|
||||
|
||||
Reference in New Issue
Block a user