Compare commits

..
Author SHA1 Message Date
ChuckBuilds fc6104f229 fix(web): stop /config/main handing out every credential it holds
The endpoint returned the raw config to anyone who could reach the port, and
this web interface has no authentication of any kind. An unauthenticated
request against a live rig 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. Anything on
that LAN could read them.

The x-secret masking the plugin config endpoints use does not reach here: this
route never consults a schema, and core keys such as github.api_token have no
schema to carry the marker. Several of the fields above *are* tagged x-secret
in their plugin's schema and were still returned in full, which is what rules
out the schema route as the fix for this endpoint.

Credential-named fields are now blanked. Matching on the name is blunt, and
for a whole-config dump that is the right default: anything named like a
credential should not leave the process, and a new plugin adding a
differently-shaped secret is covered without anyone remembering to tag it.

Blanked rather than removed, and safe to blank: POST /config/main merges into
the freshly loaded config and writes only the keys it was given, so a client
that round-trips this response cannot erase a secret it never saw. The web API
suites confirm it -- 81 passing, unchanged.

On the test that matters: the first version of this suite exercised the two
helpers and nothing else, and reverting the single line that wires the
redactor into the route passed all thirty of them. 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. The added test goes through the view function, and
it does fail on that revert.

This also corrects an earlier claim of mine. I reported that GET /api/v3/config
did not expose these values; that path 404s, so the check proved nothing. The
real route is /config/main and it exposed all of them.
2026-08-20 05:09:12 -04:00
4 changed files with 184 additions and 203 deletions
-68
View File
@@ -63,9 +63,6 @@ class StartupValidator:
if self.plugin_manager:
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
if is_valid:
@@ -77,71 +74,6 @@ class StartupValidator:
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:
"""Validate configuration files."""
try:
+143
View File
@@ -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"
-133
View File
@@ -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
+41 -2
View File
@@ -262,15 +262,54 @@ def _stop_display_service():
result['status'] = status
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'])
def get_main_config():
"""Get main configuration"""
"""Get main configuration, with credentials redacted."""
try:
if not api_v3.config_manager:
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
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:
logger.error('Unhandled exception', exc_info=True)
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500