mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-18 04:49:13 +00:00
- _redact_credentials: blank scalar descendants of objects reached
through a credential-owned list (e.g. tokens: [{"value": "secret"}])
regardless of field name -- the existing name-based walk only
protected direct dict values under a credential key, not list items.
- wifi.py: reject enabled/force/auto_enable_ap_mode values
_parse_bool_ish can't recognize (400) instead of silently treating
them as False, which could disable Wi-Fi or the radio itself.
- Starlark manifest locking: lock a stable manifest.json.lock sidecar
instead of manifest.json itself, in both the standalone route path
(_starlark_manifest_lock) and the plugin path
(StarlarkAppsPlugin._save_manifest / _update_manifest_safe).
manifest.json is replaced by an atomic rename on every write, which
swaps in a fresh inode; a lock held on the old inode does not
exclude a second locker that opens the path afresh right after the
rename and gets the new inode, so two writers could race despite
each holding "a lock". A sidecar that no write ever touches always
resolves to the same inode for every locker.
Skipped as stale: the "serialize the complete manifest
read-modify-write" finding at api_v3/__init__.py -- every standalone
handler that calls _write_starlark_manifest is already wrapped in
_starlark_manifest_lock() on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
193 lines
7.6 KiB
Python
193 lines
7.6 KiB
Python
"""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_a_list_of_bare_credentials_is_redacted():
|
|
"""A credential-named key whose value is a list of scalars, not a list
|
|
of named-field dicts, used to pass through untouched: the dict branch
|
|
recursed into `v` on its own value only, so a bare string list item had
|
|
no field name to test and fell through the base case unredacted.
|
|
"""
|
|
config = {"tokens": ["abc123", "def456"], "timezone": "America/New_York"}
|
|
out = _redact_credentials(config)
|
|
assert out["tokens"] == ["", ""]
|
|
assert out["timezone"] == "America/New_York"
|
|
|
|
|
|
def test_a_list_nested_inside_a_credential_named_container_is_also_blanked():
|
|
"""The list fix applies at any depth under a credential-named key, not
|
|
only when the list is the key's direct value.
|
|
"""
|
|
config = {"auth": {"backup_tokens": ["a", "b"], "note": "keep"}}
|
|
out = _redact_credentials(config)
|
|
assert out["auth"]["backup_tokens"] == ["", ""]
|
|
assert out["auth"]["note"] == "keep"
|
|
|
|
|
|
def test_objects_inside_a_credential_named_list_are_blanked_regardless_of_field_names():
|
|
"""A list item has no field name of its own to test against
|
|
`_CREDENTIAL_NAME_PARTS`, so an object reached through a credential-owned
|
|
list must be blanked outright rather than walked by field name -- unlike
|
|
the dict-directly-under-a-credential-key case in the test above.
|
|
"""
|
|
config = {"tokens": [{"value": "secret", "kind": "bearer"}]}
|
|
out = _redact_credentials(config)
|
|
assert out["tokens"] == [{"value": "", "kind": ""}]
|
|
|
|
|
|
def test_credential_shaped_container_with_named_fields_still_only_blanks_those():
|
|
"""Unchanged behaviour for the case the container test above already
|
|
covers: a dict whose sub-keys are semantically named fields is walked
|
|
normally, not blanket-blanked, so an ordinary field next to a
|
|
credential-named one survives.
|
|
"""
|
|
config = {"secrets": {"api_key": "k", "note": "keep", "list": ["a", "b"]}}
|
|
out = _redact_credentials(config)
|
|
assert out["secrets"]["api_key"] == ""
|
|
assert out["secrets"]["note"] == "keep"
|
|
# "list" isn't itself credential-named, so it is walked, not blanked --
|
|
# but it holds no credential-named field either, so it survives whole.
|
|
assert out["secrets"]["list"] == ["a", "b"]
|
|
|
|
|
|
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"):
|
|
# get_main_config is a route; it lives in the config module now.
|
|
response = mod.config.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"
|