Files
LEDMatrix/test/test_store_plugin_id_traversal.py
ChuckandClaude Opus 5.5 bcef1957a9 fix(security): refuse unsafe plugin ids, keep secrets private, validate request bodies (#643)
* fix(security): refuse unsafe plugin ids, keep secrets private, validate bodies

- install_from_url and the registry install's manifest rename refuse a
  plugin id that is not a single safe name (no ../ out of plugins_dir).
- Uninstall and config reset refuse core config sections and ids with
  path parts; uninstall of a plugin whose directory is gone still works.
- separate_secrets checks a field's own x-secret marker before recursing,
  so object/array secrets no longer land in config.json.
- Backup restore creates missing secrets/wifi/ytm files with mode 640;
  export skips non-object manifests and no longer collides on same-second
  exports.
- SYSTEM_FONTS includes every bundled font from BUNDLED_FONTS.
- Raw config/secrets saves and validate_request_json require a JSON object.
- A blank max_dynamic_duration_seconds keeps the stored value; other values
  are validated to 30-1800 instead of raising a 500.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(security): validate the id before install_plugin moves anything; claim backup names atomically

- install_plugin set aside plugins_dir / plugin_id before any id check, so
  "../x" moved a directory outside the plugins dir (the rollback moved it
  back, but only if the install path got that far)
- two exports finishing in the same second could both see a free name and
  the later os.replace destroyed the first archive; the name is now
  claimed with O_EXCL before the archive is swapped in

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-28 08:24:43 -04:00

95 lines
3.5 KiB
Python

"""A plugin id from a request or a downloaded manifest cannot escape plugins_dir.
install_from_url joined ``plugins_dir / plugin_id`` -- plugin_id coming from
the request body or the downloaded manifest -- then removed whatever was
there and moved the download onto it, so ``"../x"`` deleted and replaced a
directory beside plugins_dir. _install_plugin_impl did the same when it
renamed the install to the manifest's id.
"""
import json
import pytest
from src.plugin_system.store_manager import PluginStoreManager
MANIFEST = {
"id": "good-plugin", "name": "Good", "class_name": "P",
"display_modes": ["good"], "version": "1.0.0",
}
@pytest.fixture
def store(tmp_path, monkeypatch):
mgr = PluginStoreManager(
plugins_dir=str(tmp_path / "plugins"),
uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
mgr.plugins_dir.mkdir(parents=True, exist_ok=True)
monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True)
mgr.manifest = dict(MANIFEST)
def fake_clone(repo_url, target, branches):
target = type(mgr.plugins_dir)(target)
target.mkdir(parents=True, exist_ok=True)
(target / "manifest.json").write_text(json.dumps(mgr.manifest))
(target / "manager.py").write_text("class P: pass\n")
return "main"
monkeypatch.setattr(mgr, "_install_via_git", fake_clone)
return mgr
@pytest.fixture
def victim(tmp_path):
# A sibling of plugins_dir that a traversal id would target.
d = tmp_path / "victim"
d.mkdir()
(d / "keep.txt").write_text("precious")
return d
def test_install_from_url_rejects_a_traversal_id_from_the_request(store, victim):
result = store.install_from_url("https://github.com/x/y", plugin_id="../victim")
assert result["success"] is False
assert "Invalid plugin ID" in result["error"]
assert (victim / "keep.txt").read_text() == "precious"
def test_install_from_url_rejects_a_traversal_id_from_the_manifest(store, victim):
store.manifest["id"] = "../victim"
result = store.install_from_url("https://github.com/x/y")
assert result["success"] is False
assert (victim / "keep.txt").read_text() == "precious"
def test_install_from_url_still_installs_a_normal_id(store):
result = store.install_from_url("https://github.com/x/y", plugin_id="ledmatrix-weather")
assert result["success"] is True
assert (store.plugins_dir / "ledmatrix-weather" / "manifest.json").exists()
def test_registry_install_rejects_a_traversal_manifest_id(store, victim, monkeypatch):
store.manifest["id"] = "../victim"
monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: {
"id": "good-plugin", "repo": "https://github.com/x/y"})
assert store._install_plugin_impl("good-plugin") is False
assert (victim / "keep.txt").read_text() == "precious"
assert not (store.plugins_dir / "good-plugin").exists()
def test_install_plugin_rejects_a_traversal_id_before_touching_disk(store, victim, monkeypatch):
# install_plugin moves an existing plugins_dir / plugin_id aside before
# installing; for "../victim" that is a directory outside plugins_dir.
# A later rollback may move it back, so assert nothing was touched at all.
touched = []
monkeypatch.setattr(store, "_set_aside", lambda *a: touched.append("set_aside"))
monkeypatch.setattr(store, "_install_plugin_impl", lambda *a, **k: touched.append("install"))
assert store.install_plugin("../victim") is False
assert touched == []
assert (victim / "keep.txt").read_text() == "precious"