mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 18:39:06 +00:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fc6104f229 |
@@ -63,9 +63,6 @@ class StartupValidator:
|
|||||||
if self.plugin_manager:
|
if self.plugin_manager:
|
||||||
self._validate_plugins()
|
self._validate_plugins()
|
||||||
|
|
||||||
# Warn when the running systemd unit no longer matches the repo's
|
|
||||||
self._validate_systemd_units()
|
|
||||||
|
|
||||||
is_valid = len(self.errors) == 0
|
is_valid = len(self.errors) == 0
|
||||||
|
|
||||||
if is_valid:
|
if is_valid:
|
||||||
@@ -77,71 +74,6 @@ class StartupValidator:
|
|||||||
|
|
||||||
return (is_valid, self.errors.copy(), self.warnings.copy())
|
return (is_valid, self.errors.copy(), self.warnings.copy())
|
||||||
|
|
||||||
#: Units this project installs, and where each is installed to.
|
|
||||||
_UNITS = (
|
|
||||||
("systemd/ledmatrix.service", "/etc/systemd/system/ledmatrix.service"),
|
|
||||||
("systemd/ledmatrix-web.service", "/etc/systemd/system/ledmatrix-web.service"),
|
|
||||||
)
|
|
||||||
|
|
||||||
def _validate_systemd_units(self) -> None:
|
|
||||||
"""Warn when an installed unit has drifted from the repo's template.
|
|
||||||
|
|
||||||
Nothing re-applies these after the first install. `git pull` -- which is
|
|
||||||
what the web UI's update button runs -- brings a new template into the
|
|
||||||
checkout, but nothing copies it to /etc/systemd/system and nothing runs
|
|
||||||
`systemctl daemon-reload`, so the unit that actually runs is whatever
|
|
||||||
first_time_install.sh wrote on day one.
|
|
||||||
|
|
||||||
That makes every hardening added to a unit inert on existing installs.
|
|
||||||
Measured on one rig: the installed unit was thirteen days older than the
|
|
||||||
repo's and differed in content, so a MemoryMax the repo had specified
|
|
||||||
was not being enforced at all -- `systemctl show` reported
|
|
||||||
MemoryMax=infinity.
|
|
||||||
|
|
||||||
A warning rather than an error, and certainly not a silent rewrite:
|
|
||||||
editing files under /etc and restarting services is the installer's job,
|
|
||||||
not something a display process should do to a machine while it boots.
|
|
||||||
The remedy is to re-run scripts/install/install_service.sh.
|
|
||||||
"""
|
|
||||||
try:
|
|
||||||
project_root = Path(__file__).resolve().parent.parent
|
|
||||||
for template_rel, installed_path in self._UNITS:
|
|
||||||
template = project_root / template_rel
|
|
||||||
installed = Path(installed_path)
|
|
||||||
if not template.is_file() or not installed.is_file():
|
|
||||||
continue
|
|
||||||
|
|
||||||
# The template carries placeholders the installer substitutes,
|
|
||||||
# so compare the substituted form rather than the raw file.
|
|
||||||
expected = template.read_text(encoding="utf-8")
|
|
||||||
expected = expected.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
|
||||||
expected = expected.replace("__USER__", "root")
|
|
||||||
|
|
||||||
try:
|
|
||||||
actual = installed.read_text(encoding="utf-8")
|
|
||||||
except PermissionError:
|
|
||||||
continue
|
|
||||||
|
|
||||||
if self._unit_body(expected) != self._unit_body(actual):
|
|
||||||
self.warnings.append(
|
|
||||||
f"{installed.name} differs from {template_rel}; the "
|
|
||||||
"installed unit is not refreshed by an update, so "
|
|
||||||
"settings added to the template are not in effect. "
|
|
||||||
"Re-run scripts/install/install_service.sh to apply them."
|
|
||||||
)
|
|
||||||
except OSError as e:
|
|
||||||
self.logger.debug("Could not compare systemd units: %s", e)
|
|
||||||
|
|
||||||
@staticmethod
|
|
||||||
def _unit_body(text: str) -> str:
|
|
||||||
"""A unit's meaningful lines: no comments, no blanks, no ordering noise."""
|
|
||||||
lines = []
|
|
||||||
for line in text.splitlines():
|
|
||||||
line = line.strip()
|
|
||||||
if line and not line.startswith("#"):
|
|
||||||
lines.append(line)
|
|
||||||
return "\n".join(sorted(lines))
|
|
||||||
|
|
||||||
def _validate_config(self) -> None:
|
def _validate_config(self) -> None:
|
||||||
"""Validate configuration files."""
|
"""Validate configuration files."""
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -0,0 +1,143 @@
|
|||||||
|
"""GET /config/main must not hand out credentials.
|
||||||
|
|
||||||
|
The endpoint returned the raw config to anyone who could reach the port, and
|
||||||
|
this web interface has no authentication of any kind. Measured against a live
|
||||||
|
rig, an unauthenticated request returned:
|
||||||
|
|
||||||
|
github.api_token 40 chars
|
||||||
|
incoming-packages.ha_token 183 chars
|
||||||
|
jellyfin-now-playing.api_key 32 chars
|
||||||
|
ledmatrix-weather.api_key 32 chars
|
||||||
|
on-air.mqtt_password 8 chars
|
||||||
|
youtube.api_key 20 chars
|
||||||
|
youtube-stats.api_key 39 chars
|
||||||
|
|
||||||
|
A GitHub token and a Home Assistant long-lived token among them.
|
||||||
|
|
||||||
|
The x-secret masking the plugin config endpoints use does not apply here: this
|
||||||
|
endpoint never consults a schema, and core keys such as github.api_token have
|
||||||
|
no schema to carry the marker. Several of those fields *are* tagged x-secret in
|
||||||
|
their plugin's schema and were still returned in full, which is what makes the
|
||||||
|
schema route the wrong one to rely on for this endpoint.
|
||||||
|
|
||||||
|
Matching on field name is blunt. For a whole-config dump it is the right
|
||||||
|
default: anything named like a credential should not leave the process, and a
|
||||||
|
new plugin that adds a differently-shaped secret is covered without anyone
|
||||||
|
remembering to tag it.
|
||||||
|
"""
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from web_interface.blueprints.api_v3 import (
|
||||||
|
_looks_like_a_credential,
|
||||||
|
_redact_credentials,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("name", [
|
||||||
|
"password", "mqtt_password", "opensky_password", "passwd",
|
||||||
|
"api_key", "apikey", "API_KEY", "flightaware_api_key",
|
||||||
|
"token", "ha_token", "api_token", "access_token",
|
||||||
|
"secret", "client_secret", "spotify_client_secret",
|
||||||
|
"access_key", "private_key",
|
||||||
|
])
|
||||||
|
def test_credential_names_are_recognised(name):
|
||||||
|
assert _looks_like_a_credential(name)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("name", [
|
||||||
|
"timezone", "city", "brightness", "enabled", "update_interval",
|
||||||
|
"favorite_teams", "display_duration", "keyword",
|
||||||
|
])
|
||||||
|
def test_ordinary_names_are_left_alone(name):
|
||||||
|
assert not _looks_like_a_credential(name)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_measured_leak_is_closed():
|
||||||
|
"""The exact shape taken off the rig."""
|
||||||
|
config = {
|
||||||
|
"github": {"api_token": "ghp_" + "x" * 36},
|
||||||
|
"incoming-packages": {"ha_token": "y" * 183, "enabled": True},
|
||||||
|
"jellyfin-now-playing": {"api_key": "z" * 32},
|
||||||
|
"on-air": {"mqtt_password": "hunter22"},
|
||||||
|
"youtube": {"api_key": "k" * 20},
|
||||||
|
"timezone": "America/New_York",
|
||||||
|
}
|
||||||
|
out = _redact_credentials(config)
|
||||||
|
assert out["github"]["api_token"] == ""
|
||||||
|
assert out["incoming-packages"]["ha_token"] == ""
|
||||||
|
assert out["jellyfin-now-playing"]["api_key"] == ""
|
||||||
|
assert out["on-air"]["mqtt_password"] == ""
|
||||||
|
assert out["youtube"]["api_key"] == ""
|
||||||
|
# Everything else survives, or the config editor breaks.
|
||||||
|
assert out["timezone"] == "America/New_York"
|
||||||
|
assert out["incoming-packages"]["enabled"] is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_nested_and_listed_credentials_are_reached():
|
||||||
|
config = {"a": {"b": {"c": {"password": "p"}}},
|
||||||
|
"feeds": [{"name": "x", "api_key": "k"}, {"name": "y"}]}
|
||||||
|
out = _redact_credentials(config)
|
||||||
|
assert out["a"]["b"]["c"]["password"] == ""
|
||||||
|
assert out["feeds"][0]["api_key"] == ""
|
||||||
|
assert out["feeds"][0]["name"] == "x"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_original_is_not_mutated():
|
||||||
|
"""The caller holds the live config; redaction must not edit it in place."""
|
||||||
|
config = {"github": {"api_token": "keepme"}}
|
||||||
|
_redact_credentials(config)
|
||||||
|
assert config["github"]["api_token"] == "keepme"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_credential_shaped_container_is_still_walked():
|
||||||
|
"""`secrets: {...}` is a section name, not a value to blank."""
|
||||||
|
config = {"secrets": {"api_key": "k", "note": "keep"}}
|
||||||
|
out = _redact_credentials(config)
|
||||||
|
assert out["secrets"]["api_key"] == ""
|
||||||
|
assert out["secrets"]["note"] == "keep"
|
||||||
|
|
||||||
|
|
||||||
|
def test_non_dict_input_passes_through():
|
||||||
|
assert _redact_credentials("plain") == "plain"
|
||||||
|
assert _redact_credentials(7) == 7
|
||||||
|
assert _redact_credentials(None) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_endpoint_itself_redacts():
|
||||||
|
"""Through the view function, not the helper.
|
||||||
|
|
||||||
|
The helper tests above all passed with the route still returning
|
||||||
|
`config` -- reverting the one line that calls the redactor changed
|
||||||
|
nothing, because nothing exercised the route. A property asserted on a
|
||||||
|
helper is not a property asserted on the endpoint, and it is the endpoint
|
||||||
|
that is exposed to the network.
|
||||||
|
"""
|
||||||
|
import json as _json
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import flask
|
||||||
|
|
||||||
|
from web_interface.blueprints import api_v3 as mod
|
||||||
|
|
||||||
|
raw = {"github": {"api_token": "ghp_secret_value"},
|
||||||
|
"timezone": "America/New_York"}
|
||||||
|
|
||||||
|
manager = MagicMock()
|
||||||
|
manager.load_config.return_value = raw
|
||||||
|
previous = getattr(mod.api_v3, "config_manager", None)
|
||||||
|
mod.api_v3.config_manager = manager
|
||||||
|
|
||||||
|
app = flask.Flask(__name__)
|
||||||
|
try:
|
||||||
|
with app.test_request_context("/config/main"):
|
||||||
|
response = mod.get_main_config()
|
||||||
|
payload = response.get_json() if hasattr(response, "get_json") else _json.loads(response[0].data)
|
||||||
|
finally:
|
||||||
|
mod.api_v3.config_manager = previous
|
||||||
|
|
||||||
|
data = payload["data"]
|
||||||
|
assert data["github"]["api_token"] == "", (
|
||||||
|
"the endpoint returned the token; the redactor is not wired in")
|
||||||
|
assert data["timezone"] == "America/New_York"
|
||||||
|
# And the config the manager handed over is untouched.
|
||||||
|
assert raw["github"]["api_token"] == "ghp_secret_value"
|
||||||
@@ -1,133 +0,0 @@
|
|||||||
"""An installed unit that no longer matches the repo's must be reported.
|
|
||||||
|
|
||||||
Nothing re-applies systemd units after the first install. `git pull` -- what
|
|
||||||
the web UI's update button runs -- brings a new template into the checkout, but
|
|
||||||
no code in web_interface/ or src/ copies it to /etc/systemd/system or runs
|
|
||||||
`systemctl daemon-reload`. The unit that actually runs is whatever
|
|
||||||
first_time_install.sh wrote on day one.
|
|
||||||
|
|
||||||
So every hardening added to a unit is inert on existing installs. Measured on a
|
|
||||||
live rig: the installed unit was dated 2026-08-06 and the repo's 2026-08-19,
|
|
||||||
and they differed -- with the result that a MemoryMax=85% present in the repo's
|
|
||||||
template was not being enforced at all. `systemctl show` reported
|
|
||||||
MemoryMax=infinity.
|
|
||||||
|
|
||||||
This is a warning, not an error, and deliberately not a silent rewrite:
|
|
||||||
editing files under /etc and restarting services is the installer's job, not
|
|
||||||
something a display process should do to a machine while it boots.
|
|
||||||
"""
|
|
||||||
import logging
|
|
||||||
from pathlib import Path
|
|
||||||
from unittest.mock import MagicMock
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from src.startup_validator import StartupValidator
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
|
||||||
def validator():
|
|
||||||
v = StartupValidator(config_manager=MagicMock())
|
|
||||||
v.logger = logging.getLogger("test")
|
|
||||||
v.warnings = []
|
|
||||||
v.errors = []
|
|
||||||
return v
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_matching_unit_produces_no_warning(validator, tmp_path):
|
|
||||||
"""The installed unit, substituted exactly as the installer would."""
|
|
||||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
|
||||||
template_rel = "systemd/ledmatrix.service"
|
|
||||||
template = project_root / template_rel
|
|
||||||
if not template.is_file():
|
|
||||||
pytest.skip("repo unit template not present")
|
|
||||||
|
|
||||||
installed = tmp_path / "ledmatrix.service"
|
|
||||||
installed.write_text(
|
|
||||||
template.read_text(encoding="utf-8")
|
|
||||||
.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
|
||||||
.replace("__USER__", "root"),
|
|
||||||
encoding="utf-8")
|
|
||||||
|
|
||||||
validator._UNITS = ((template_rel, str(installed)),)
|
|
||||||
validator._validate_systemd_units()
|
|
||||||
assert not validator.warnings, f"a matching unit warned: {validator.warnings}"
|
|
||||||
assert not validator.errors
|
|
||||||
|
|
||||||
|
|
||||||
def test_comments_and_blank_lines_are_not_drift():
|
|
||||||
"""Otherwise every comment the repo adds would look like a changed unit."""
|
|
||||||
a = "[Service]\n# explains a setting\nExecStart=/x\nRestart=always\n"
|
|
||||||
b = "[Service]\nExecStart=/x\n\nRestart=always\n"
|
|
||||||
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_changed_directive_is_drift():
|
|
||||||
a = "[Service]\nExecStart=/x\nMemoryMax=85%\n"
|
|
||||||
b = "[Service]\nExecStart=/x\n"
|
|
||||||
assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b)
|
|
||||||
|
|
||||||
|
|
||||||
def test_reordered_directives_are_not_drift():
|
|
||||||
"""systemd does not care about order within a section, so neither should this."""
|
|
||||||
a = "[Service]\nExecStart=/x\nRestart=always\n"
|
|
||||||
b = "[Service]\nRestart=always\nExecStart=/x\n"
|
|
||||||
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
|
|
||||||
|
|
||||||
|
|
||||||
def test_cosmetic_differences_do_not_warn(validator, tmp_path):
|
|
||||||
"""Through the real comparison, not the helper.
|
|
||||||
|
|
||||||
The repo's template carries explanatory comments the installed copy may not
|
|
||||||
have, and the installer does not preserve ordering or blank lines. If those
|
|
||||||
counted as drift, every boot would warn and the warning would be ignored.
|
|
||||||
Asserting this on _unit_body alone would not catch a comparison that stopped
|
|
||||||
calling it -- which is exactly what a careless edit does.
|
|
||||||
"""
|
|
||||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
|
||||||
template_rel = "systemd/ledmatrix.service"
|
|
||||||
template = project_root / template_rel
|
|
||||||
if not template.is_file():
|
|
||||||
pytest.skip("repo unit template not present")
|
|
||||||
|
|
||||||
substituted = (template.read_text(encoding="utf-8")
|
|
||||||
.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
|
||||||
.replace("__USER__", "root"))
|
|
||||||
# Same directives, stripped of comments and blank lines and reordered.
|
|
||||||
directives = sorted(line.strip() for line in substituted.splitlines()
|
|
||||||
if line.strip() and not line.strip().startswith("#"))
|
|
||||||
installed = tmp_path / "ledmatrix.service"
|
|
||||||
installed.write_text("\n".join(reversed(directives)) + "\n", encoding="utf-8")
|
|
||||||
|
|
||||||
validator._UNITS = ((template_rel, str(installed)),)
|
|
||||||
validator._validate_systemd_units()
|
|
||||||
assert not validator.warnings, (
|
|
||||||
f"cosmetic-only difference reported as drift: {validator.warnings}")
|
|
||||||
|
|
||||||
|
|
||||||
def test_drift_is_reported_as_a_warning(validator, tmp_path):
|
|
||||||
"""The whole point: a real difference must surface, and only as a warning."""
|
|
||||||
installed = tmp_path / "ledmatrix.service"
|
|
||||||
installed.write_text("[Service]\nExecStart=/usr/bin/python3 /x/run.py\n")
|
|
||||||
|
|
||||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
|
||||||
template_rel = "systemd/ledmatrix.service"
|
|
||||||
template = project_root / template_rel
|
|
||||||
if not template.is_file():
|
|
||||||
pytest.skip("repo unit template not present")
|
|
||||||
|
|
||||||
validator._UNITS = ((template_rel, str(installed)),)
|
|
||||||
validator._validate_systemd_units()
|
|
||||||
|
|
||||||
assert validator.warnings, "a differing unit produced no warning"
|
|
||||||
assert "install_service.sh" in validator.warnings[0], (
|
|
||||||
"the warning does not tell the user how to fix it")
|
|
||||||
assert not validator.errors, "drift must not be fatal at startup"
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_missing_installed_unit_is_silent(validator, tmp_path):
|
|
||||||
"""Development checkouts have no /etc/systemd unit; that is not drift."""
|
|
||||||
validator._UNITS = (("systemd/ledmatrix.service", str(tmp_path / "absent.service")),)
|
|
||||||
validator._validate_systemd_units()
|
|
||||||
assert not validator.warnings
|
|
||||||
assert not validator.errors
|
|
||||||
@@ -262,15 +262,54 @@ def _stop_display_service():
|
|||||||
result['status'] = status
|
result['status'] = status
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
#: Field names whose value is a credential. Matched by name because this
|
||||||
|
#: endpoint returns the whole config, core keys included, and core config has
|
||||||
|
#: no schema to carry x-secret markers.
|
||||||
|
_CREDENTIAL_NAME_PARTS = ("password", "passwd", "secret", "token", "api_key",
|
||||||
|
"apikey", "access_key", "private_key", "client_secret")
|
||||||
|
|
||||||
|
|
||||||
|
def _looks_like_a_credential(name: str) -> bool:
|
||||||
|
lowered = name.lower()
|
||||||
|
return any(part in lowered for part in _CREDENTIAL_NAME_PARTS)
|
||||||
|
|
||||||
|
|
||||||
|
def _redact_credentials(value):
|
||||||
|
"""A copy of `value` with credential-named fields blanked.
|
||||||
|
|
||||||
|
/config/main returned the raw config to anyone who could reach the port,
|
||||||
|
and this interface has no authentication. On one rig that meant a 40-char
|
||||||
|
GitHub token, a 183-char Home Assistant token and five API keys were
|
||||||
|
readable by anything on the LAN.
|
||||||
|
|
||||||
|
The x-secret masking used by the plugin config endpoints does not help
|
||||||
|
here: this endpoint never consults a schema, and core keys such as
|
||||||
|
github.api_token have no schema to mark. Matching on the field name is
|
||||||
|
blunt, but for a whole-config dump the right default is that anything
|
||||||
|
named like a credential does not leave the process.
|
||||||
|
|
||||||
|
Blanked rather than removed, and safe to blank: POST /config/main merges
|
||||||
|
into the loaded config and only writes the keys it was given, so a client
|
||||||
|
that round-trips this response cannot erase a secret it never saw.
|
||||||
|
"""
|
||||||
|
if isinstance(value, dict):
|
||||||
|
return {k: ("" if _looks_like_a_credential(k) and not isinstance(v, (dict, list))
|
||||||
|
else _redact_credentials(v))
|
||||||
|
for k, v in value.items()}
|
||||||
|
if isinstance(value, list):
|
||||||
|
return [_redact_credentials(item) for item in value]
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
@api_v3.route('/config/main', methods=['GET'])
|
@api_v3.route('/config/main', methods=['GET'])
|
||||||
def get_main_config():
|
def get_main_config():
|
||||||
"""Get main configuration"""
|
"""Get main configuration, with credentials redacted."""
|
||||||
try:
|
try:
|
||||||
if not api_v3.config_manager:
|
if not api_v3.config_manager:
|
||||||
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
|
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
|
||||||
|
|
||||||
config = api_v3.config_manager.load_config()
|
config = api_v3.config_manager.load_config()
|
||||||
return jsonify({'status': 'success', 'data': config})
|
return jsonify({'status': 'success', 'data': _redact_credentials(config)})
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error('Unhandled exception', exc_info=True)
|
logger.error('Unhandled exception', exc_info=True)
|
||||||
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
|
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
|
||||||
|
|||||||
Reference in New Issue
Block a user