mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-20 18:09:05 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f0efdafbdb |
@@ -1,143 +0,0 @@
|
||||
"""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"
|
||||
@@ -262,54 +262,15 @@ 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, with credentials redacted."""
|
||||
"""Get main configuration"""
|
||||
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': _redact_credentials(config)})
|
||||
return jsonify({'status': 'success', 'data': 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
|
||||
@@ -754,10 +715,12 @@ def save_main_config():
|
||||
if not data:
|
||||
return jsonify({'status': 'error', 'message': 'No data provided'}), 400
|
||||
|
||||
import logging
|
||||
logging.error(f"DEBUG: save_main_config received data: {data}")
|
||||
logging.error(f"DEBUG: Content-Type header: {request.content_type}")
|
||||
logging.error(f"DEBUG: Headers: {dict(request.headers)}")
|
||||
# What arrives here is the config itself, and the headers carry the
|
||||
# session cookie -- neither belongs in the journal, least of all at
|
||||
# ERROR on every save. The shape of the request is the part with
|
||||
# diagnostic value, so log that, at the level it deserves.
|
||||
logger.debug("save_main_config: %s, %d top-level key(s)",
|
||||
request.content_type or 'no content-type', len(data))
|
||||
|
||||
# Merge with existing config (similar to original implementation)
|
||||
current_config = api_v3.config_manager.load_config()
|
||||
|
||||
Reference in New Issue
Block a user