diff --git a/assets/sports/ncaa_logos/COR.png b/assets/sports/ncaa_logos/COR.png new file mode 100644 index 00000000..d39027fa Binary files /dev/null and b/assets/sports/ncaa_logos/COR.png differ diff --git a/src/web_interface/secret_helpers.py b/src/web_interface/secret_helpers.py index 310d9848..f4cae945 100644 --- a/src/web_interface/secret_helpers.py +++ b/src/web_interface/secret_helpers.py @@ -143,6 +143,12 @@ def mask_secret_fields(config: Dict[str, Any], schema_properties: Dict[str, Any] return result +#: What a masked secret looks like on the wire. Named because the write path +#: has to recognise it coming back: a client that renders the mask and posts +#: it unchanged must not store the mask as if it were the secret. +SECRET_MASK = '\u2022' * 8 + + def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]: """Blanket-mask every non-empty value in a secrets config dict. @@ -161,7 +167,7 @@ def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]: if isinstance(v, dict): masked[k] = mask_all_secret_values(v) elif v not in (None, '') and not (isinstance(v, str) and v.startswith('YOUR_')): - masked[k] = '••••••••' + masked[k] = SECRET_MASK else: masked[k] = v return masked @@ -189,3 +195,30 @@ def remove_empty_secrets(secrets: Dict[str, Any]) -> Dict[str, Any]: elif v is not None and not (isinstance(v, str) and v.strip() == ''): result[k] = v return result + + +def strip_masked_values(secrets: Dict[str, Any]) -> Dict[str, Any]: + """Remove values a client echoed back rather than changed. + + The counterpart to :func:`mask_all_secret_values`. A client that GETs the + masked secrets, edits one field and POSTs the whole object back is sending + ``SECRET_MASK`` for every field it did not touch. Storing those would + replace each untouched credential with eight bullet characters. + + Drops the mask and, like :func:`remove_empty_secrets`, blank values -- so + the caller can merge the result onto what is already stored and have + "unchanged" mean unchanged. Empty nested dicts are pruned. + """ + result: Dict[str, Any] = {} + for k, v in secrets.items(): + if isinstance(v, dict): + nested = strip_masked_values(v) + if nested: + result[k] = nested + elif v is None: + continue + elif isinstance(v, str) and (v.strip() == '' or v == SECRET_MASK): + continue + else: + result[k] = v + return result diff --git a/test/web_interface/test_config_secrets_masking.py b/test/web_interface/test_config_secrets_masking.py new file mode 100644 index 00000000..dcae66c8 --- /dev/null +++ b/test/web_interface/test_config_secrets_masking.py @@ -0,0 +1,82 @@ +"""GET /config/secrets must not hand out credentials, and the client's +read-modify-write cycle must not destroy them. + +This interface has no authentication. The endpoint returned the whole +config_secrets.json to anyone who could reach the port; on one rig that was a +40-character GitHub token, a 183-character Home Assistant token and three API +keys. Masking it alone is not enough: the only client fetches every secret, +edits one field and posts all of them back, so the write path has to treat an +echoed mask as "unchanged". +""" +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent)) + +from test_api_v3_secret_roundtrip import env, _on_disk # noqa: F401,E402 +from src.web_interface.secret_helpers import SECRET_MASK # noqa: E402 + +STORED = { + "github": {"api_token": "ghp_" + "x" * 36}, + "ledmatrix-weather": {"api_key": "w" * 32}, + "incoming-packages": {"ha_token": "h" * 183}, + "unset-plugin": {"api_key": ""}, + "placeholder-plugin": {"api_key": "YOUR_API_KEY_HERE"}, +} + + +def _seed(env): + env.secrets_file.write_text(json.dumps(STORED)) + + +def _get(env): + r = env.client.get("/api/v3/config/secrets") + assert r.status_code == 200, r.get_data(as_text=True)[:200] + return r.get_json()["data"] + + +def test_no_credential_leaves_the_process(env): + _seed(env) + body = json.dumps(_get(env)) + for secret in ("ghp_" + "x" * 36, "w" * 32, "h" * 183): + assert secret not in body, "endpoint returned a stored credential" + + +def test_set_and_unset_remain_distinguishable(env): + _seed(env) + data = _get(env) + assert data["github"]["api_token"] == SECRET_MASK + assert data["unset-plugin"]["api_key"] == "" + assert data["placeholder-plugin"]["api_key"] == "YOUR_API_KEY_HERE" + + +def test_the_clients_read_modify_write_preserves_every_other_secret(env): + """What the GitHub-token save button actually does.""" + _seed(env) + secrets = _get(env) # everything arrives masked + secrets["github"]["api_token"] = "ghp_" + "n" * 36 # user changes one + r = env.client.post("/api/v3/config/raw/secrets", json=secrets) + assert r.status_code == 200, r.get_data(as_text=True)[:200] + + on_disk = _on_disk(env.secrets_file) + assert on_disk["github"]["api_token"] == "ghp_" + "n" * 36, "new token not saved" + assert on_disk["ledmatrix-weather"]["api_key"] == "w" * 32 + assert on_disk["incoming-packages"]["ha_token"] == "h" * 183 + + +def test_a_mask_echoed_back_is_never_stored(env): + _seed(env) + env.client.post("/api/v3/config/raw/secrets", json=_get(env)) + on_disk = _on_disk(env.secrets_file) + assert SECRET_MASK not in json.dumps(on_disk), "the mask was stored as a secret" + assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36 + + +def test_a_brand_new_secret_can_still_be_added(env): + _seed(env) + env.client.post("/api/v3/config/raw/secrets", + json={"new-plugin": {"api_key": "brand-new"}}) + on_disk = _on_disk(env.secrets_file) + assert on_disk["new-plugin"]["api_key"] == "brand-new" + assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36 diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 984637e8..83fb3386 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -21,7 +21,8 @@ logger = logging.getLogger(__name__) # Import new infrastructure from src.web_interface.api_helpers import success_response, error_response, validate_request_json from src.web_interface.errors import ErrorCode -from src.web_interface.secret_helpers import find_secret_fields, separate_secrets +from src.web_interface.secret_helpers import (find_secret_fields, mask_all_secret_values, + separate_secrets, strip_masked_values) from src.web_interface.error_handler import describe_exception, redact_text from src.plugin_system.operation_types import OperationType from src.web_interface.validators import ( @@ -1333,7 +1334,12 @@ def get_secrets_config(): return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 config = api_v3.config_manager.get_raw_file_content('secrets') - return jsonify({'status': 'success', 'data': config}) + # This interface has no authentication, and this file is nothing but + # credentials. It was handing all of them to anyone who could reach + # the port. Values are masked; empty and YOUR_* placeholders are left + # alone so a client can still tell "set" from "not set". + return jsonify({'status': 'success', + 'data': mask_all_secret_values(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 @@ -1395,8 +1401,19 @@ def save_raw_secrets_config(): if not data: return jsonify({'status': 'error', 'message': 'No data provided'}), 400 - # Save the secrets config - api_v3.config_manager.save_raw_file_content('secrets', data) + # The GET above masks what it returns, and this endpoint's only client + # reads the whole file, edits one field and posts all of it back. So + # most of what arrives here is the mask, echoed rather than changed -- + # storing it verbatim would replace every untouched credential with + # eight bullets. Strip those, then merge onto what is already stored, + # which makes "unchanged" mean unchanged. + # + # The cost is that a secret can no longer be cleared by blanking it. + # That needs its own affordance; a control that erases credentials as + # a side effect of saving an unrelated one is not it. + current = api_v3.config_manager.get_raw_file_content('secrets') or {} + merged = deep_merge(current, strip_masked_values(data)) + api_v3.config_manager.save_raw_file_content('secrets', merged) # Reload GitHub token in plugin store manager if it exists if api_v3.plugin_store_manager: diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index 8bca1821..ea020998 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -4622,15 +4622,17 @@ window.loadGithubToken = function() { // Handle empty data (secrets file doesn't exist) - API returns {} in this case const secrets = data.data || {}; const token = secrets.github?.api_token || ''; + const configured = token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN'; if (input) { - if (token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN') { - // Token exists and is valid - input.value = token; - showNotification('GitHub token loaded successfully', 'success'); + // The endpoint masks what it returns, so this never holds + // the real token -- and the field is deliberately left + // empty rather than filled with the mask, which would be + // saved verbatim the next time the user pressed Save. + input.value = ''; + if (configured) { + showNotification('A GitHub token is saved. Enter a new one to replace it.', 'success'); } else { - // No token configured or placeholder value - input.value = ''; showNotification('No GitHub token configured. Enter a new token to save.', 'info'); } }