mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
* fix(security): refuse unsafe plugin ids, keep secrets private, validate bodies - install_from_url and the registry install's manifest rename refuse a plugin id that is not a single safe name (no ../ out of plugins_dir). - Uninstall and config reset refuse core config sections and ids with path parts; uninstall of a plugin whose directory is gone still works. - separate_secrets checks a field's own x-secret marker before recursing, so object/array secrets no longer land in config.json. - Backup restore creates missing secrets/wifi/ytm files with mode 640; export skips non-object manifests and no longer collides on same-second exports. - SYSTEM_FONTS includes every bundled font from BUNDLED_FONTS. - Raw config/secrets saves and validate_request_json require a JSON object. - A blank max_dynamic_duration_seconds keeps the stored value; other values are validated to 30-1800 instead of raising a 500. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(security): validate the id before install_plugin moves anything; claim backup names atomically - install_plugin set aside plugins_dir / plugin_id before any id check, so "../x" moved a directory outside the plugins dir (the rollback moved it back, but only if the install path got that far) - two exports finishing in the same second could both see a free name and the later os.replace destroyed the first archive; the name is now claimed with O_EXCL before the archive is swapped in Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
324 lines
14 KiB
Python
324 lines
14 KiB
Python
"""
|
|
Tests for src/web_interface/secret_helpers.py — the canonical secret
|
|
identification / separation / masking helpers.
|
|
|
|
This module is the extracted single source of truth for x-secret handling,
|
|
but until now had zero test coverage (only ``mask_secret_fields`` is even
|
|
imported by production code, from pages_v3). api_v3.py still carries three
|
|
inline re-implementations of ``find_secret_fields``/``separate_secrets`` —
|
|
see test_secret_separation_parity.py — so pinning the canonical behavior
|
|
here is a precondition for ever migrating those copies.
|
|
"""
|
|
|
|
import copy
|
|
|
|
from src.web_interface.secret_helpers import (
|
|
find_secret_fields,
|
|
separate_secrets,
|
|
mask_secret_fields,
|
|
mask_all_secret_values,
|
|
merge_secrets,
|
|
remove_empty_secrets,
|
|
)
|
|
|
|
|
|
SCHEMA_PROPS = {
|
|
"api_key": {"type": "string", "x-secret": True},
|
|
"city": {"type": "string"},
|
|
"auth": {
|
|
"type": "object",
|
|
"properties": {
|
|
"token": {"type": "string", "x-secret": True},
|
|
"username": {"type": "string"},
|
|
},
|
|
},
|
|
"accounts": {
|
|
"type": "array",
|
|
"items": {
|
|
"type": "object",
|
|
"properties": {
|
|
"name": {"type": "string"},
|
|
"token": {"type": "string", "x-secret": True},
|
|
},
|
|
},
|
|
},
|
|
"recovery_codes": {
|
|
"type": "array",
|
|
"items": {"type": "string", "x-secret": True},
|
|
},
|
|
}
|
|
|
|
|
|
class TestFindSecretFields:
|
|
def test_top_level_secret(self):
|
|
assert "api_key" in find_secret_fields(SCHEMA_PROPS)
|
|
|
|
def test_non_secret_not_included(self):
|
|
assert "city" not in find_secret_fields(SCHEMA_PROPS)
|
|
|
|
def test_nested_object_secret_uses_dot_path(self):
|
|
assert "auth.token" in find_secret_fields(SCHEMA_PROPS)
|
|
assert "auth.username" not in find_secret_fields(SCHEMA_PROPS)
|
|
|
|
def test_array_item_object_secret_uses_bracket_path(self):
|
|
assert "accounts[].token" in find_secret_fields(SCHEMA_PROPS)
|
|
|
|
def test_array_of_secrets_uses_bracket_path(self):
|
|
assert "recovery_codes[]" in find_secret_fields(SCHEMA_PROPS)
|
|
|
|
def test_full_set(self):
|
|
assert find_secret_fields(SCHEMA_PROPS) == {
|
|
"api_key", "auth.token", "accounts[].token", "recovery_codes[]",
|
|
}
|
|
|
|
def test_non_dict_properties_tolerated(self):
|
|
assert find_secret_fields({"weird": "not-a-dict"}) == set()
|
|
|
|
def test_non_dict_input_returns_empty(self):
|
|
assert find_secret_fields(None) == set()
|
|
assert find_secret_fields([]) == set()
|
|
|
|
|
|
class TestSeparateSecrets:
|
|
def test_flat_partition(self):
|
|
regular, secrets = separate_secrets(
|
|
{"api_key": "s3cret", "city": "Austin"}, {"api_key"})
|
|
assert regular == {"city": "Austin"}
|
|
assert secrets == {"api_key": "s3cret"}
|
|
|
|
def test_nested_partition(self):
|
|
config = {"auth": {"token": "t0k", "username": "chuck"}}
|
|
regular, secrets = separate_secrets(config, {"auth.token"})
|
|
assert regular == {"auth": {"username": "chuck"}}
|
|
assert secrets == {"auth": {"token": "t0k"}}
|
|
|
|
def test_empty_nested_dicts_pruned_from_regular(self):
|
|
# A dict that is all secrets leaves nothing behind on the regular
|
|
# side — the key must be dropped, not kept as {}.
|
|
config = {"auth": {"token": "t0k"}}
|
|
regular, secrets = separate_secrets(config, {"auth.token"})
|
|
assert regular == {}
|
|
assert secrets == {"auth": {"token": "t0k"}}
|
|
|
|
def test_whole_array_secret(self):
|
|
config = {"recovery_codes": ["a", "b"], "city": "Austin"}
|
|
regular, secrets = separate_secrets(config, {"recovery_codes[]"})
|
|
assert regular == {"city": "Austin"}
|
|
assert secrets == {"recovery_codes": ["a", "b"]}
|
|
|
|
def test_array_item_secrets_produce_parallel_lists(self):
|
|
# Per-item secrets keep the arrays index-aligned so they can be
|
|
# recombined: regular gets the stripped items, secrets a parallel
|
|
# list of the extracted values.
|
|
config = {"accounts": [
|
|
{"name": "a", "token": "ta"},
|
|
{"name": "b", "token": "tb"},
|
|
]}
|
|
regular, secrets = separate_secrets(config, {"accounts[].token"})
|
|
assert regular == {"accounts": [{"name": "a"}, {"name": "b"}]}
|
|
assert secrets == {"accounts": [{"token": "ta"}, {"token": "tb"}]}
|
|
|
|
def test_array_item_non_dict_items_get_placeholder(self):
|
|
config = {"accounts": [{"name": "a", "token": "ta"}, "oddball"]}
|
|
regular, secrets = separate_secrets(config, {"accounts[].token"})
|
|
assert regular == {"accounts": [{"name": "a"}, "oddball"]}
|
|
assert secrets == {"accounts": [{"token": "ta"}, {}]}
|
|
|
|
def test_secret_field_holding_an_object_or_array_goes_to_secrets(self):
|
|
# x-secret on the field itself, not its children: the dict/list type
|
|
# check used to win, and the whole value landed in config.json.
|
|
props = {
|
|
"oauth": {"type": "object", "x-secret": True},
|
|
"cookies": {"type": "array", "x-secret": True},
|
|
"city": {"type": "string"},
|
|
}
|
|
config = {"oauth": {"refresh": "r3fr3sh"}, "cookies": ["c1", "c2"],
|
|
"city": "Austin"}
|
|
regular, secrets = separate_secrets(config, find_secret_fields(props))
|
|
assert regular == {"city": "Austin"}
|
|
assert secrets == {"oauth": {"refresh": "r3fr3sh"}, "cookies": ["c1", "c2"]}
|
|
# ...which is what the API already masks for those fields.
|
|
masked = mask_secret_fields(config, props)
|
|
assert masked["oauth"] == "" and masked["cookies"] == ""
|
|
|
|
def test_array_without_secret_paths_stays_regular(self):
|
|
config = {"teams": ["DAL", "HOU"]}
|
|
regular, secrets = separate_secrets(config, {"api_key"})
|
|
assert regular == {"teams": ["DAL", "HOU"]}
|
|
assert secrets == {}
|
|
|
|
def test_round_trip_loses_nothing(self):
|
|
# separate + naive recombine must reconstruct the original config.
|
|
config = {
|
|
"api_key": "k",
|
|
"city": "Austin",
|
|
"auth": {"token": "t", "username": "chuck"},
|
|
"recovery_codes": ["a", "b"],
|
|
}
|
|
paths = find_secret_fields(SCHEMA_PROPS)
|
|
regular, secrets = separate_secrets(copy.deepcopy(config), paths)
|
|
|
|
def recombine(reg, sec):
|
|
out = copy.deepcopy(reg)
|
|
for k, v in sec.items():
|
|
if isinstance(v, dict) and isinstance(out.get(k), dict):
|
|
out[k] = recombine(out[k], v)
|
|
else:
|
|
out[k] = v
|
|
return out
|
|
|
|
assert recombine(regular, secrets) == config
|
|
|
|
|
|
class TestMaskSecretFields:
|
|
def test_masks_present_secret_to_empty_string(self):
|
|
result = mask_secret_fields({"api_key": "s3cret"}, SCHEMA_PROPS)
|
|
assert result["api_key"] == ""
|
|
|
|
def test_leaves_non_secret_untouched(self):
|
|
result = mask_secret_fields({"city": "Austin"}, SCHEMA_PROPS)
|
|
assert result["city"] == "Austin"
|
|
|
|
def test_none_and_empty_left_alone(self):
|
|
result = mask_secret_fields({"api_key": None}, SCHEMA_PROPS)
|
|
assert result["api_key"] is None
|
|
result = mask_secret_fields({"api_key": ""}, SCHEMA_PROPS)
|
|
assert result["api_key"] == ""
|
|
|
|
def test_falsey_but_set_values_are_masked(self):
|
|
# 0 and False are real values; the check is `is not None and != ''`.
|
|
# Note False == '' is False in Python, so False IS masked; 0 == '' is
|
|
# also False, so 0 is masked too.
|
|
result = mask_secret_fields({"api_key": 0}, SCHEMA_PROPS)
|
|
assert result["api_key"] == ""
|
|
result = mask_secret_fields({"api_key": False}, SCHEMA_PROPS)
|
|
assert result["api_key"] == ""
|
|
|
|
def test_nested_object_masked_without_mutating_input(self):
|
|
config = {"auth": {"token": "t0k", "username": "chuck"}}
|
|
original = copy.deepcopy(config)
|
|
result = mask_secret_fields(config, SCHEMA_PROPS)
|
|
assert result["auth"]["token"] == ""
|
|
assert result["auth"]["username"] == "chuck"
|
|
assert config == original # input not mutated
|
|
|
|
def test_array_of_secrets_masked_elementwise(self):
|
|
result = mask_secret_fields(
|
|
{"recovery_codes": ["a", "b"]}, SCHEMA_PROPS)
|
|
assert result["recovery_codes"] == ["", ""]
|
|
|
|
def test_array_of_objects_masked_per_item(self):
|
|
config = {"accounts": [{"name": "a", "token": "ta"}, "oddball"]}
|
|
result = mask_secret_fields(config, SCHEMA_PROPS)
|
|
assert result["accounts"][0] == {"name": "a", "token": ""}
|
|
assert result["accounts"][1] == "oddball"
|
|
|
|
def test_non_dict_schema_property_tolerated(self):
|
|
assert mask_secret_fields({"x": 1}, {"x": "bogus"}) == {"x": 1}
|
|
|
|
|
|
class TestMaskAllSecretValues:
|
|
def test_real_values_replaced_with_bullets(self):
|
|
assert mask_all_secret_values({"key": "abc"}) == {"key": "••••••••"}
|
|
|
|
def test_placeholders_preserved(self):
|
|
# YOUR_* placeholders must survive so the UI can show "not set".
|
|
result = mask_all_secret_values({"key": "YOUR_API_KEY_HERE"})
|
|
assert result == {"key": "YOUR_API_KEY_HERE"}
|
|
|
|
def test_empty_and_none_preserved(self):
|
|
assert mask_all_secret_values({"a": "", "b": None}) == {"a": "", "b": None}
|
|
|
|
def test_recurses_into_nested_dicts(self):
|
|
result = mask_all_secret_values({"plugin": {"token": "t", "empty": ""}})
|
|
assert result == {"plugin": {"token": "••••••••", "empty": ""}}
|
|
|
|
def test_non_string_real_values_masked(self):
|
|
assert mask_all_secret_values({"port": 8080}) == {"port": "••••••••"}
|
|
|
|
|
|
class TestRemoveEmptySecrets:
|
|
def test_strips_empty_string(self):
|
|
assert remove_empty_secrets({"a": "", "b": "real"}) == {"b": "real"}
|
|
|
|
def test_strips_whitespace_only(self):
|
|
assert remove_empty_secrets({"a": " "}) == {}
|
|
|
|
def test_strips_none(self):
|
|
assert remove_empty_secrets({"a": None}) == {}
|
|
|
|
def test_prunes_empty_nested_dicts(self):
|
|
assert remove_empty_secrets({"plugin": {"token": ""}}) == {}
|
|
|
|
def test_keeps_nested_real_values(self):
|
|
result = remove_empty_secrets({"plugin": {"token": "t", "empty": ""}})
|
|
assert result == {"plugin": {"token": "t"}}
|
|
|
|
def test_keeps_falsey_non_string_values(self):
|
|
# 0 and False are neither None nor blank strings — they are kept.
|
|
assert remove_empty_secrets({"a": 0, "b": False}) == {"a": 0, "b": False}
|
|
|
|
|
|
class TestArrayItemSecrets:
|
|
"""Lists merge by replacement, so a blanked array wipes stored credentials.
|
|
|
|
remove_empty_secrets recursed into dicts but let a list through untouched,
|
|
so [{"token": ""}] went straight into deep_merge and overwrote the stored
|
|
list. Saving any unrelated setting destroyed every token in the array.
|
|
"""
|
|
|
|
STORED = {"accounts": [{"name": "a", "token": "REAL-A"},
|
|
{"name": "b", "token": "REAL-B"}]}
|
|
|
|
def test_an_unrelated_save_keeps_every_stored_token(self):
|
|
posted = {"accounts": [{"name": "a", "token": ""},
|
|
{"name": "b", "token": ""}]}
|
|
merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
|
|
assert [a["token"] for a in merged["accounts"]] == ["REAL-A", "REAL-B"]
|
|
|
|
def test_editing_one_entry_leaves_the_others_alone(self):
|
|
posted = {"accounts": [{"name": "a", "token": ""},
|
|
{"name": "b", "token": "NEW-B"}]}
|
|
merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
|
|
assert [a["token"] for a in merged["accounts"]] == ["REAL-A", "NEW-B"]
|
|
|
|
def test_a_new_entry_is_appended(self):
|
|
posted = {"accounts": [{"name": "a", "token": ""},
|
|
{"name": "b", "token": ""},
|
|
{"name": "c", "token": "NEW-C"}]}
|
|
merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
|
|
assert [a["token"] for a in merged["accounts"]] == \
|
|
["REAL-A", "REAL-B", "NEW-C"]
|
|
|
|
def test_a_list_of_bare_strings_merges_by_index(self):
|
|
merged = merge_secrets({"keys": ["K1", "K2", "K3"]},
|
|
remove_empty_secrets({"keys": ["", "K2-NEW", ""]}))
|
|
assert merged["keys"] == ["K1", "K2-NEW", "K3"]
|
|
|
|
def test_an_all_blank_list_is_dropped_entirely(self):
|
|
posted = {"accounts": [{"token": ""}, {"token": ""}]}
|
|
assert "accounts" not in remove_empty_secrets(posted)
|
|
|
|
def test_plain_dict_secrets_are_unaffected(self):
|
|
merged = merge_secrets({"api_key": "OLD", "other": "keep"},
|
|
remove_empty_secrets({"api_key": "", "other": "changed"}))
|
|
assert merged == {"api_key": "OLD", "other": "changed"}
|
|
|
|
def test_a_removed_entry_takes_its_secret_with_it(self):
|
|
"""The regular config's list is authoritative about how many items
|
|
exist, and the secrets list runs parallel to it -- see
|
|
ConfigManager._strip_secrets_recursive. So a shorter incoming list
|
|
must shorten the stored secrets too, or the two fall out of step."""
|
|
posted = {"accounts": [{"name": "a", "token": "NEW-A"}]}
|
|
merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
|
|
assert [a["token"] for a in merged["accounts"]] == ["NEW-A"]
|
|
|
|
def test_an_emptied_item_stays_a_dict_not_none(self):
|
|
"""None there stops the list looking parallel, and
|
|
_strip_secrets_recursive then drops the whole key from the main
|
|
config -- deleting the item's non-secret fields as well."""
|
|
pruned = remove_empty_secrets(
|
|
{"accounts": [{"token": "real"}, {"token": ""}]})
|
|
assert pruned["accounts"] == [{"token": "real"}, {}]
|
|
assert None not in pruned["accounts"]
|