From a025f9897fcef446848552478abe41c5f7d054ef Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:50:23 -0400 Subject: [PATCH] fix(web): pass plugin action params to the wrapper on stdin POST /api/v3/plugins/action runs a plugin's script through a generated Python wrapper, and the params went into that wrapper's source as `params = `. JSON true, false and null are undefined names in Python, so any params holding one made the wrapper die with a NameError before the script ran, and the route answered "Action failed". The plugin file manager's category toggle sends {"category_name": ..., "enabled": true}, so of-the-day's category toggle failed every time. The wrapper now reads the params from its own stdin (json.loads) and the route passes them there; nothing taken from the request is written into the generated source any more. The script's side is unchanged: the same json.dumps(params) on its stdin, LEDMATRIX_ROOT set, stdout parsed. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 ++ test/test_api_v3_plugin_action_params.py | 93 ++++++++++++++++++++++ web_interface/blueprints/api_v3/plugins.py | 9 ++- 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 test/test_api_v3_plugin_action_params.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fa3a386..77ae8b1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -549,6 +549,14 @@ policies are unchanged. screen showed first and the game came after it. Each check also asks each plugin `has_live_content()` once, where a plugin registered under several modes used to be asked once per mode. +- A plugin action whose params hold `true`, `false` or `null` runs again. + `/api/v3/plugins/action` wrote the params into the source of the wrapper + that runs the plugin's script, and those JSON words are not Python, so the + wrapper stopped with a NameError and the action answered "Action failed". + The plugin file manager's category toggle sends `"enabled": true`, so + turning a category on or off in of-the-day always failed. The params now + reach the wrapper on its stdin; the script still receives them as JSON on + its own stdin, as before. - The display schedule turns the panel off at exactly the end time. A window now runs from its start time up to, but not including, its end time: with 07:00-23:00 the panel is on at 07:00 and off at 23:00. Before, the end diff --git a/test/test_api_v3_plugin_action_params.py b/test/test_api_v3_plugin_action_params.py new file mode 100644 index 00000000..a890290a --- /dev/null +++ b/test/test_api_v3_plugin_action_params.py @@ -0,0 +1,93 @@ +"""POST /api/v3/plugins/action hands ``params`` to the plugin's script intact. + +The route runs the script through a generated wrapper, and the params went +into that wrapper as Python source: ``params = {json.dumps(params)}``. JSON is +not Python. ``true``, ``false`` and ``null`` are undefined names there, so any +params holding a boolean or a null died with a NameError before the script +ran. The plugin file manager's category toggle sends ``{"category_name": ..., +"enabled": true}``, so of-the-day's category toggle failed every time with +"Action failed". + +The script's side of the contract is unchanged and pinned here too: the +params arrive on stdin as one JSON document, LEDMATRIX_ROOT is set, and what +the script prints to stdout is what the route parses. +""" + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + +ACTION_URL = "/api/v3/plugins/action" + +# The action script: report what it was handed, as JSON on stdout. +ECHO_SCRIPT = ( + "import json, os, sys\n" + "raw = sys.stdin.read()\n" + "print(json.dumps({'status': 'success', 'got': json.loads(raw),\n" + " 'root': os.environ.get('LEDMATRIX_ROOT')}))\n" +) + + +@pytest.fixture +def echo_plugin(tmp_path, api_v3_module, monkeypatch): + plugin_dir = tmp_path / "demo" + plugin_dir.mkdir() + (plugin_dir / "manifest.json").write_text(json.dumps({ + "id": "demo", + "web_ui_actions": [{"id": "toggle", "type": "script", "script": "echo.py"}], + }), encoding="utf-8") + (plugin_dir / "echo.py").write_text(ECHO_SCRIPT, encoding="utf-8") + api_v3_module.api_v3.plugin_catalog.get_plugin_directory.return_value = str(plugin_dir) + + # The route runs `python3`; use this interpreter, so the test does not + # depend on what that name resolves to here. + real_run = subprocess.run + + def run(cmd, *args, **kwargs): + if isinstance(cmd, list) and cmd and cmd[0] == "python3": + cmd = [sys.executable] + cmd[1:] + return real_run(cmd, *args, **kwargs) + + monkeypatch.setattr(subprocess, "run", run) + return plugin_dir + + +@pytest.mark.parametrize("params", [ + {"category_name": "jokes", "enabled": True}, # the file manager's toggle + {"category_name": "jokes", "enabled": False}, + {"filename": None}, + {"nested": {"list": [1, None, True, 2.5], "empty": {}}}, + {"text": "café ✓ \U0001F600"}, + {"text": "he said \"hi\" and 'bye' \\ ''' \"\"\" \n\t end"}, +], ids=["true", "false", "null", "nested", "unicode", "quotes"]) +def test_the_script_receives_the_params_it_was_sent(api_v3_client, echo_plugin, params): + response = api_v3_client.post(ACTION_URL, json={ + "plugin_id": "demo", "action_id": "toggle", "params": params}) + body = response.get_json() + assert response.status_code == 200, body + assert body["got"] == params + + +def test_a_param_cannot_run_code_in_the_wrapper(api_v3_client, echo_plugin, tmp_path): + marker = tmp_path / "PWNED" + hostile = "\"}\nopen(%r, 'w').write('ran')\n#" % str(marker) + params = {"name": hostile, "flag": True} + response = api_v3_client.post(ACTION_URL, json={ + "plugin_id": "demo", "action_id": "toggle", "params": params}) + assert response.status_code == 200, response.get_json() + assert response.get_json()["got"] == params + assert not marker.exists(), "a param value ran as code" + + +def test_the_script_still_gets_ledmatrix_root(api_v3_client, echo_plugin, api_v3_module): + response = api_v3_client.post(ACTION_URL, json={ + "plugin_id": "demo", "action_id": "toggle", "params": {"enabled": True}}) + assert response.status_code == 200, response.get_json() + assert response.get_json()["root"] == str(api_v3_module.PROJECT_ROOT) diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index 8c6bd2a3..180efe9a 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -430,6 +430,10 @@ sys.exit(proc.returncode) import tempfile import json as json_lib + # The params reach the wrapper on its stdin, never in + # its source: written there as `params = `, a + # true, false or null was an undefined name and the + # wrapper died with a NameError before the script ran. params_json = json_lib.dumps(action_params) with tempfile.NamedTemporaryFile(mode='w', suffix='.py', delete=False) as wrapper: wrapper.write(f'''import sys @@ -440,6 +444,9 @@ import json # Set LEDMATRIX_ROOT os.environ['LEDMATRIX_ROOT'] = r"{PROJECT_ROOT}" +# The params, as JSON on this wrapper's own stdin +params = json.loads(sys.stdin.read()) + # Run the script and provide params as JSON via stdin proc = subprocess.Popen( [sys.executable, r"{script_file}"], @@ -451,7 +458,6 @@ proc = subprocess.Popen( ) # Send params as JSON to stdin -params = {params_json} stdout, _ = proc.communicate(input=json.dumps(params), timeout=120) print(stdout) sys.exit(proc.returncode) @@ -461,6 +467,7 @@ sys.exit(proc.returncode) try: result = subprocess.run( ['python3', wrapper_path], + input=params_json, capture_output=True, text=True, timeout=120,