mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 18:39:06 +00:00
Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 ChuckandClaude Opus 5
|
9c0c0dc851 | ||
|
ChuckandClaude Opus 5
|
71739d85d1 | ||
|
ChuckandClaude Opus 5
|
cc258aaffd |
Binary file not shown.
|
After Width: | Height: | Size: 467 B |
@@ -18,7 +18,7 @@ tooling against it.
|
||||
| `web_display_autostart` | bool, `true` | Whether the web interface service starts with the system | `scripts/utils/start_web_conditionally.py` |
|
||||
| `timezone` | string, `"America/New_York"` | IANA timezone for schedules and displays | `ConfigManager.get_timezone()` |
|
||||
| `target_fps` | int, `100` | Frame-rate ceiling for plugin rendering | `src/plugin_system/base_plugin.py`, `src/common/sports_scroll.py` |
|
||||
| `location` | object | `city` / `state` / `country`. Supplies the **default** for a plugin's own `location_city` / `location_state` / `location_country` setting, so weather, radar and friends follow this device without being configured twice. A value saved on the plugin itself still overrides it. | `SchemaManager.apply_device_location()`, then plugins via merged config |
|
||||
| `location` | object | `city` / `state` / `country`, offered to plugins that need a location (weather, etc.) | plugins via merged config |
|
||||
|
||||
## `schedule` — display on/off hours
|
||||
|
||||
|
||||
Regular → Executable
@@ -100,10 +100,15 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service"
|
||||
|
||||
# Optional: journalctl (non-critical — skip if not found)
|
||||
#
|
||||
# NOEXEC, matching first_time_install.sh. These rules end in a wildcard and
|
||||
# journalctl starts a pager, so without it the caller can reach a shell:
|
||||
# less runs "!command" as the user the pager belongs to, which here is
|
||||
# root. NOEXEC stops the granted command executing anything of its own.
|
||||
if [ -n "$JOURNALCTL_PATH" ]; then
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix.service *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -t ledmatrix *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *"
|
||||
fi
|
||||
|
||||
# Required: python3, bash
|
||||
|
||||
@@ -25,9 +25,44 @@ if [ "$EUID" -eq 0 ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Resolve command paths against a fixed PATH, and check what we resolved.
|
||||
#
|
||||
# Every path found here is written into a sudoers file as a NOPASSWD grant, so
|
||||
# whoever controls the binary at that path controls root. first_time_install.sh
|
||||
# re-execs itself with `sudo -E`, which preserves the invoking user's
|
||||
# environment -- PATH included -- so without pinning it, `which nmcli` can
|
||||
# resolve to anything on that PATH: a writable directory early in it turns a
|
||||
# compromise of the low-privilege web user into permanent root.
|
||||
PATH=/usr/sbin:/usr/bin:/sbin:/bin
|
||||
export PATH
|
||||
|
||||
# A binary named in a sudoers rule must be root-owned and writable by nobody
|
||||
# else, or the grant hands root to whoever can rewrite it.
|
||||
require_trusted_binary() {
|
||||
local label="$1" path="$2"
|
||||
if [ ! -x "$path" ]; then
|
||||
echo "✗ $label: $path is not an executable file"
|
||||
exit 1
|
||||
fi
|
||||
local owner perms
|
||||
owner=$(stat -c '%u' "$path") || exit 1
|
||||
perms=$(stat -c '%a' "$path") || exit 1
|
||||
if [ "$owner" != "0" ]; then
|
||||
echo "✗ $label: $path is not owned by root (uid $owner); refusing to"
|
||||
echo " grant it NOPASSWD sudo."
|
||||
exit 1
|
||||
fi
|
||||
# Group- or world-writable means someone other than root can replace it.
|
||||
case "$perms" in
|
||||
*[2367]) echo "✗ $label: $path is writable by group or other ($perms);"
|
||||
echo " refusing to grant it NOPASSWD sudo."
|
||||
exit 1 ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# Get the full paths to commands
|
||||
NMCLI_PATH=$(which nmcli || echo "/usr/bin/nmcli")
|
||||
SYSTEMCTL_PATH=$(which systemctl)
|
||||
NMCLI_PATH=$(command -v nmcli || echo "/usr/bin/nmcli")
|
||||
SYSTEMCTL_PATH=$(command -v systemctl)
|
||||
|
||||
echo "Command paths:"
|
||||
echo " nmcli: $NMCLI_PATH"
|
||||
@@ -37,6 +72,18 @@ echo " systemctl: $SYSTEMCTL_PATH"
|
||||
echo ""
|
||||
echo "Step 1: Configuring sudo permissions for nmcli..."
|
||||
SUDOERS_FILE="/etc/sudoers.d/ledmatrix_wifi"
|
||||
SYSCTL_PATH=$(command -v sysctl || echo /usr/sbin/sysctl)
|
||||
NFT_PATH=$(command -v nft || echo /usr/sbin/nft)
|
||||
RFKILL_PATH=$(command -v rfkill || echo /usr/sbin/rfkill)
|
||||
MKDIR_PATH=$(command -v mkdir || echo /usr/bin/mkdir)
|
||||
|
||||
# Checked before any of them reaches the sudoers file.
|
||||
require_trusted_binary "nmcli" "$NMCLI_PATH"
|
||||
require_trusted_binary "systemctl" "$SYSTEMCTL_PATH"
|
||||
require_trusted_binary "sysctl" "$SYSCTL_PATH"
|
||||
require_trusted_binary "nft" "$NFT_PATH"
|
||||
require_trusted_binary "rfkill" "$RFKILL_PATH"
|
||||
require_trusted_binary "mkdir" "$MKDIR_PATH"
|
||||
|
||||
# Create a temporary sudoers file using mktemp (handles permissions better)
|
||||
TEMP_SUDOERS=$(mktemp) || {
|
||||
@@ -62,6 +109,36 @@ $WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start dnsmasq
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop dnsmasq
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart dnsmasq
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart NetworkManager
|
||||
# The captive portal turns IP forwarding on while the access point is up and
|
||||
# restores the previous value when it comes down (wifi_manager._setup_iptables_
|
||||
# redirect / _teardown_iptables_redirect). Without this rule that sudo call
|
||||
# needs a password, so forwarding stays off and clients associate to the AP but
|
||||
# cannot route. It goes unnoticed on a stock Raspberry Pi image, where
|
||||
# /etc/sudoers.d/010_pi-nopasswd grants the default user blanket NOPASSWD and
|
||||
# masks every gap in this file -- it only bites once that blanket rule is
|
||||
# removed.
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSCTL_PATH -w net.ipv4.ip_forward=0
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSCTL_PATH -w net.ipv4.ip_forward=1
|
||||
# The portal's redirect lives in its own nftables table, created when the AP
|
||||
# comes up and deleted when it goes down, and the radio has to be unblocked
|
||||
# before the AP can start at all. Same story as the sysctl rules above: called
|
||||
# with sudo, never granted here, and invisible on a stock Pi image.
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $NFT_PATH add table ip ledmatrix
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $NFT_PATH delete table ip ledmatrix
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $RFKILL_PATH unblock wifi
|
||||
# NetworkManager's dnsmasq drop-in directory, exact path.
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $MKDIR_PATH -p /etc/NetworkManager/dnsmasq-shared.d
|
||||
#
|
||||
# iptables is deliberately NOT granted here. Its rules are built from the live
|
||||
# interface name and port, so a rule covering them needs a trailing wildcard --
|
||||
# and `iptables --modprobe=/path/to/anything` runs that path as root, so
|
||||
# `NOPASSWD: iptables *` is a root shell for the web user by another name. That
|
||||
# is a worse outcome than the gap it would close, which today is masked anyway
|
||||
# by the blanket NOPASSWD rule on stock Pi images.
|
||||
#
|
||||
# Closing it safely means a wrapper script that builds the rules itself and
|
||||
# takes only an interface and a port, granted the way safe_plugin_rm.sh already
|
||||
# is. That belongs in its own change rather than being smuggled into this one.
|
||||
|
||||
# Allow copying hostapd and dnsmasq config files into place
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: /usr/bin/cp /tmp/hostapd.conf /etc/hostapd/hostapd.conf
|
||||
|
||||
Regular → Executable
Regular → Executable
@@ -71,8 +71,7 @@ class PluginManager:
|
||||
self.plugin_loader = PluginLoader(logger=self.logger)
|
||||
self.plugin_executor = PluginExecutor(default_timeout=30.0, logger=self.logger)
|
||||
self.state_manager = PluginStateManager(logger=self.logger)
|
||||
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger,
|
||||
config_manager=self.config_manager)
|
||||
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger)
|
||||
|
||||
# Lock protecting plugin_manifests and plugin_directories from
|
||||
# concurrent mutation (background reconciliation) and reads (requests).
|
||||
|
||||
@@ -26,25 +26,7 @@ class SchemaManager:
|
||||
- Cache invalidation on plugin changes
|
||||
"""
|
||||
|
||||
# Plugin config keys that mean "where this device is". A plugin declaring
|
||||
# any of these in its schema gets the device-wide ``location`` block from
|
||||
# config.json as the *default* for that field, instead of whatever city the
|
||||
# plugin author happened to ship. A value the user set on the plugin itself
|
||||
# always wins -- this only ever replaces the schema default, so an explicit
|
||||
# per-plugin location is still honoured.
|
||||
#
|
||||
# Only these fully-namespaced keys are substituted. A bare ``state`` or
|
||||
# ``city`` key is deliberately left alone: plugins use those for unrelated
|
||||
# things (ledmatrix-elections' ``state`` is a two-letter code, not a place
|
||||
# name), and silently rewriting them would break those plugins.
|
||||
DEVICE_LOCATION_KEYS: Dict[str, str] = {
|
||||
'location_city': 'city',
|
||||
'location_state': 'state',
|
||||
'location_country': 'country',
|
||||
}
|
||||
|
||||
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None,
|
||||
logger: Optional[logging.Logger] = None, config_manager: Optional[Any] = None):
|
||||
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None, logger: Optional[logging.Logger] = None):
|
||||
"""
|
||||
Initialize the Schema Manager.
|
||||
|
||||
@@ -52,14 +34,10 @@ class SchemaManager:
|
||||
plugins_dir: Base plugins directory path
|
||||
project_root: Project root directory path
|
||||
logger: Optional logger instance
|
||||
config_manager: Optional config manager, used to resolve the
|
||||
device-wide ``location`` that seeds plugin location defaults.
|
||||
Omitting it simply leaves schema defaults untouched.
|
||||
"""
|
||||
self.logger = logger or logging.getLogger(__name__)
|
||||
self.plugins_dir = plugins_dir
|
||||
self.project_root = project_root or Path.cwd()
|
||||
self.config_manager = config_manager
|
||||
|
||||
# Schema cache: plugin_id -> schema dict
|
||||
self._schema_cache: Dict[str, Dict[str, Any]] = {}
|
||||
@@ -234,70 +212,10 @@ class SchemaManager:
|
||||
|
||||
return defaults
|
||||
|
||||
def get_device_location(self) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
Return the device-wide ``location`` block from config.json, or None.
|
||||
|
||||
This is the City/State/Country the user sets once under General
|
||||
settings. Returns None when there is no config manager wired, the
|
||||
config can't be read, or no location has been configured.
|
||||
"""
|
||||
if self.config_manager is None:
|
||||
return None
|
||||
try:
|
||||
config = self.config_manager.load_config()
|
||||
except Exception as e:
|
||||
# A config that can't be read must never stop defaults being
|
||||
# generated -- the plugin's own schema defaults still apply.
|
||||
self.logger.debug(f"Could not read device location from config: {e}")
|
||||
return None
|
||||
if not isinstance(config, dict):
|
||||
return None
|
||||
location = config.get('location')
|
||||
return location if isinstance(location, dict) else None
|
||||
|
||||
def apply_device_location(self, defaults: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""
|
||||
Replace location-shaped schema defaults with the device's own location.
|
||||
|
||||
Without this, a plugin that ships ``"location_city": "Dallas"`` as its
|
||||
schema default silently reports Dallas weather (and centres its radar
|
||||
there) for every user who never opened that plugin's config form --
|
||||
even though they set their real city under General settings. The
|
||||
substituted value is still only a *default*: ``merge_with_defaults``
|
||||
lets any per-plugin value the user saved win over it.
|
||||
|
||||
Mutates and returns ``defaults`` for convenience.
|
||||
"""
|
||||
if not defaults:
|
||||
return defaults
|
||||
if not any(key in defaults for key in self.DEVICE_LOCATION_KEYS):
|
||||
return defaults
|
||||
|
||||
location = self.get_device_location()
|
||||
if not location:
|
||||
return defaults
|
||||
|
||||
for key, field in self.DEVICE_LOCATION_KEYS.items():
|
||||
if key not in defaults:
|
||||
continue
|
||||
value = location.get(field)
|
||||
# Only a non-empty string is a real answer; a blank or missing
|
||||
# field means "not configured", which leaves the schema default.
|
||||
if isinstance(value, str) and value.strip():
|
||||
defaults[key] = value.strip()
|
||||
|
||||
return defaults
|
||||
|
||||
def generate_default_config(self, plugin_id: str, use_cache: bool = True) -> Dict[str, Any]:
|
||||
"""
|
||||
Generate default configuration for a plugin from its schema.
|
||||
|
||||
Location fields (see ``DEVICE_LOCATION_KEYS``) default to the device's
|
||||
configured location rather than the plugin author's. That substitution
|
||||
is applied on the way out rather than being cached, so changing the
|
||||
device location takes effect without invalidating the defaults cache.
|
||||
|
||||
Args:
|
||||
plugin_id: Plugin identifier
|
||||
use_cache: If True, return cached defaults if available
|
||||
@@ -307,7 +225,7 @@ class SchemaManager:
|
||||
"""
|
||||
# Check cache first
|
||||
if use_cache and plugin_id in self._defaults_cache:
|
||||
return self.apply_device_location(self._defaults_cache[plugin_id].copy())
|
||||
return self._defaults_cache[plugin_id].copy()
|
||||
|
||||
schema = self.load_schema(plugin_id, use_cache=use_cache)
|
||||
if not schema:
|
||||
@@ -331,11 +249,10 @@ class SchemaManager:
|
||||
if 'live_priority' not in defaults:
|
||||
defaults['live_priority'] = schema.get('properties', {}).get('live_priority', {}).get('default', False)
|
||||
|
||||
# Cache the defaults *before* the device location is layered on, so a
|
||||
# later change to the device location is picked up by the next call.
|
||||
# Cache the defaults
|
||||
self._defaults_cache[plugin_id] = defaults.copy()
|
||||
|
||||
return self.apply_device_location(defaults)
|
||||
return defaults
|
||||
|
||||
def validate_config_against_schema(self, config: Dict[str, Any], schema: Dict[str, Any],
|
||||
plugin_id: Optional[str] = None) -> Tuple[bool, List[str]]:
|
||||
|
||||
@@ -63,6 +63,9 @@ 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:
|
||||
@@ -74,6 +77,80 @@ 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, in order: no comments, no blanks.
|
||||
|
||||
Order is preserved deliberately. This used to sort, which made the
|
||||
comparison insensitive to two changes that matter in a systemd unit:
|
||||
repeated directives such as ExecStartPre= and ExecStartPost= run in
|
||||
the order they appear, and a directive that moves between [Unit],
|
||||
[Service] and [Install] means something different -- or nothing --
|
||||
where it lands. A drift check that normalises those away reports no
|
||||
drift for a unit that has genuinely changed.
|
||||
"""
|
||||
lines = []
|
||||
for line in text.splitlines():
|
||||
line = line.strip()
|
||||
if line and not line.startswith("#"):
|
||||
lines.append(line)
|
||||
return "\n".join(lines)
|
||||
|
||||
def _validate_config(self) -> None:
|
||||
"""Validate configuration files."""
|
||||
try:
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -156,15 +162,25 @@ def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
Returns:
|
||||
A copy with all real values replaced by ``'••••••••'``.
|
||||
"""
|
||||
masked: Dict[str, Any] = {}
|
||||
for k, v in config.items():
|
||||
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] = '••••••••'
|
||||
else:
|
||||
masked[k] = v
|
||||
return masked
|
||||
return {k: _mask_value(v) for k, v in config.items()}
|
||||
|
||||
|
||||
def _mask_value(value: Any) -> Any:
|
||||
"""Mask one value, recursing through dicts and lists.
|
||||
|
||||
A list used to be masked as though it were a scalar, so
|
||||
``accounts: [{"name": "a", "token": "..."}]`` came back as a single
|
||||
``'••••••••'``. Nothing leaked, but the caller could no longer see how
|
||||
many entries there were or any of their non-secret fields, and the raw
|
||||
editor was shown a string where the file holds an array.
|
||||
"""
|
||||
if isinstance(value, dict):
|
||||
return {k: _mask_value(v) for k, v in value.items()}
|
||||
if isinstance(value, list):
|
||||
return [_mask_value(item) for item in value]
|
||||
if value in (None, '') or (isinstance(value, str) and value.startswith('YOUR_')):
|
||||
return value
|
||||
return SECRET_MASK
|
||||
|
||||
|
||||
def remove_empty_secrets(secrets: Dict[str, Any]) -> Dict[str, Any]:
|
||||
@@ -189,3 +205,52 @@ 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 isinstance(v, list):
|
||||
# A list is merged by replacement, not element by element -- there
|
||||
# is no identity to match entries on -- so a list that still holds
|
||||
# a mask cannot be merged safely: keeping it would store bullets,
|
||||
# and keeping the submitted entries alone would drop whichever the
|
||||
# client did not send back. Dropping the key leaves the stored
|
||||
# list untouched, which is what an untouched list should do.
|
||||
#
|
||||
# The consequence, deliberately: editing one secret inside a list
|
||||
# through this endpoint requires sending real values for all of
|
||||
# them. Sending some masks leaves the whole list as it was.
|
||||
if not _contains_mask(v):
|
||||
result[k] = v
|
||||
elif v is None:
|
||||
continue
|
||||
elif isinstance(v, str) and (v.strip() == '' or v == SECRET_MASK):
|
||||
continue
|
||||
else:
|
||||
result[k] = v
|
||||
return result
|
||||
|
||||
|
||||
def _contains_mask(value: Any) -> bool:
|
||||
"""True when a mask sentinel survives anywhere inside ``value``."""
|
||||
if isinstance(value, dict):
|
||||
return any(_contains_mask(v) for v in value.values())
|
||||
if isinstance(value, list):
|
||||
return any(_contains_mask(item) for item in value)
|
||||
return value == SECRET_MASK
|
||||
|
||||
Regular → Executable
Regular → Executable
@@ -8,6 +8,18 @@ Type=simple
|
||||
User=root
|
||||
WorkingDirectory=__PROJECT_ROOT_DIR__
|
||||
Environment=PYTHONDONTWRITEBYTECODE=1
|
||||
# glibc gives each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||
# and an arena that has grown is never handed back to the OS. This process runs
|
||||
# 9 threads on a 3-core Pi, so the ceiling is 24 arenas -- and a rig measured at
|
||||
# 1030 MB resident held 23 large anonymous mappings on 64 MB-aligned addresses,
|
||||
# 920 MB of them, while the live data it was actually holding (widest scroll
|
||||
# strip seen: 35,746 x 64) accounts for roughly 15 MB. That gap is arena bloat,
|
||||
# not leaked objects: RSS was flat across repeated sampling, not climbing.
|
||||
#
|
||||
# Capping the arenas trades a little allocator concurrency for a large amount of
|
||||
# resident memory on a device that has neither to spare. 2 is the usual value;
|
||||
# raise it if frame times regress.
|
||||
Environment=MALLOC_ARENA_MAX=2
|
||||
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
|
||||
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
|
||||
# for a reason that no longer applies, e.g. a config reload) would otherwise leave
|
||||
|
||||
@@ -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"
|
||||
@@ -58,7 +58,7 @@ def repos(tmp_path):
|
||||
def test_branch_with_upstream_uses_a_plain_pull(repos):
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase']
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash']
|
||||
assert note == ''
|
||||
|
||||
|
||||
@@ -73,7 +73,7 @@ def test_branch_without_upstream_falls_back_to_origin_branch(repos):
|
||||
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase', 'origin', 'audit']
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash', 'origin', 'audit']
|
||||
assert 'audit' in note
|
||||
|
||||
|
||||
@@ -155,7 +155,7 @@ def test_switching_attaches_tracking_so_pull_needs_no_fallback(repos):
|
||||
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase']
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash']
|
||||
assert note == ''
|
||||
|
||||
|
||||
@@ -200,3 +200,44 @@ def test_stash_option_lets_the_switch_through_and_keeps_the_work(repos):
|
||||
assert _git('branch', '--show-current', cwd=repos).stdout.strip() == 'other'
|
||||
# The edit is not lost — it is on the stash.
|
||||
assert 'switch to other' in _git('stash', 'list', cwd=repos).stdout
|
||||
|
||||
|
||||
class TestInstallerDoesNotBlockTheUpdateButton:
|
||||
"""first_time_install.sh chmods scripts that git tracked as 644.
|
||||
|
||||
With core.fileMode true -- the default on Linux -- that leaves five
|
||||
permanently modified tracked files on every machine that ran the
|
||||
installer, and `git pull --rebase` refuses to start:
|
||||
|
||||
error: cannot pull with rebase: You have unstaged changes.
|
||||
|
||||
Tracking them as executable makes the installer's chmod a no-op.
|
||||
"""
|
||||
|
||||
CHMODDED = [
|
||||
'first_time_install.sh',
|
||||
'start_display.sh',
|
||||
'stop_display.sh',
|
||||
'scripts/install/install_service.sh',
|
||||
'scripts/install/install_web_service.sh',
|
||||
]
|
||||
|
||||
def test_scripts_the_installer_chmods_are_tracked_executable(self):
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
root = Path(__file__).resolve().parent.parent
|
||||
out = subprocess.run(['git', 'ls-files', '-s', *self.CHMODDED],
|
||||
capture_output=True, text=True, cwd=str(root)).stdout
|
||||
modes = {line.split()[3]: line.split()[0] for line in out.strip().split('\n') if line}
|
||||
# git ls-files says nothing about a path it does not track, so a
|
||||
# renamed or deleted script would simply be absent here and the mode
|
||||
# check below would pass over it silently.
|
||||
untracked = sorted(set(self.CHMODDED) - set(modes))
|
||||
assert not untracked, (
|
||||
f"{untracked} are chmodded by the installer but not tracked by "
|
||||
"git, so their mode cannot be asserted at all")
|
||||
non_exec = sorted(f for f, m in modes.items() if m != '100755')
|
||||
assert not non_exec, (
|
||||
f"{non_exec} are chmodded by the installer but tracked non-executable, "
|
||||
"so every install leaves the working tree dirty and the update "
|
||||
"button cannot pull")
|
||||
|
||||
@@ -1,179 +0,0 @@
|
||||
"""
|
||||
Tests for the device-location default: a plugin that ships a location field in
|
||||
its schema must default to the device's configured City/State/Country, not to
|
||||
whatever place the plugin author hard-coded.
|
||||
|
||||
The bug this pins: ledmatrix-weather ships ``"location_city": "Dallas"`` as a
|
||||
schema default, so a user who set Kansas City under General settings but never
|
||||
opened the weather plugin's own config form got Dallas weather — and a radar
|
||||
centred on Dallas — with nothing in config.json to explain it.
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from src.plugin_system.schema_manager import SchemaManager
|
||||
|
||||
|
||||
class FakeConfigManager:
|
||||
"""Minimal stand-in exposing the load_config() SchemaManager relies on."""
|
||||
|
||||
def __init__(self, config):
|
||||
self.config = config
|
||||
self.load_count = 0
|
||||
|
||||
def load_config(self):
|
||||
self.load_count += 1
|
||||
return self.config
|
||||
|
||||
|
||||
class ExplodingConfigManager:
|
||||
def load_config(self):
|
||||
raise OSError("config.json is unreadable")
|
||||
|
||||
|
||||
WEATHER_SCHEMA = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"location_city": {"type": "string", "default": "Dallas"},
|
||||
"location_state": {"type": "string", "default": "Texas"},
|
||||
"location_country": {"type": "string", "default": "US"},
|
||||
"units": {"type": "string", "default": "imperial"},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def write_plugin(plugins_dir, plugin_id, schema):
|
||||
plugin_dir = plugins_dir / plugin_id
|
||||
plugin_dir.mkdir(parents=True, exist_ok=True)
|
||||
(plugin_dir / "config_schema.json").write_text(json.dumps(schema))
|
||||
return plugin_dir
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def plugins_dir(tmp_path):
|
||||
d = tmp_path / "plugin-repos"
|
||||
d.mkdir()
|
||||
return d
|
||||
|
||||
|
||||
def make_sm(plugins_dir, tmp_path, location):
|
||||
config = {} if location is None else {"location": location}
|
||||
cm = FakeConfigManager(config)
|
||||
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
|
||||
config_manager=cm)
|
||||
return sm, cm
|
||||
|
||||
|
||||
class TestDeviceLocationDefaults:
|
||||
def test_device_location_replaces_plugin_default(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, _ = make_sm(plugins_dir, tmp_path,
|
||||
{"city": "Kansas City", "state": "Missouri", "country": "US"})
|
||||
|
||||
defaults = sm.generate_default_config("ledmatrix-weather")
|
||||
|
||||
assert defaults["location_city"] == "Kansas City"
|
||||
assert defaults["location_state"] == "Missouri"
|
||||
assert defaults["location_country"] == "US"
|
||||
# Non-location defaults are untouched.
|
||||
assert defaults["units"] == "imperial"
|
||||
|
||||
def test_user_set_plugin_value_still_wins(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, _ = make_sm(plugins_dir, tmp_path,
|
||||
{"city": "Kansas City", "state": "Missouri", "country": "US"})
|
||||
|
||||
defaults = sm.generate_default_config("ledmatrix-weather")
|
||||
merged = sm.merge_with_defaults({"location_city": "Denver"}, defaults)
|
||||
|
||||
assert merged["location_city"] == "Denver"
|
||||
# Fields the user did not override still follow the device.
|
||||
assert merged["location_state"] == "Missouri"
|
||||
|
||||
def test_blank_and_missing_device_fields_leave_schema_default(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, _ = make_sm(plugins_dir, tmp_path, {"city": "Kansas City", "state": " "})
|
||||
|
||||
defaults = sm.generate_default_config("ledmatrix-weather")
|
||||
|
||||
assert defaults["location_city"] == "Kansas City"
|
||||
assert defaults["location_state"] == "Texas" # blank -> not configured
|
||||
assert defaults["location_country"] == "US" # absent -> schema default
|
||||
|
||||
def test_no_device_location_configured_is_a_no_op(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, _ = make_sm(plugins_dir, tmp_path, None)
|
||||
|
||||
defaults = sm.generate_default_config("ledmatrix-weather")
|
||||
|
||||
assert defaults["location_city"] == "Dallas"
|
||||
|
||||
def test_no_config_manager_is_a_no_op(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path)
|
||||
|
||||
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
|
||||
|
||||
def test_unreadable_config_falls_back_to_schema_defaults(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
|
||||
config_manager=ExplodingConfigManager())
|
||||
|
||||
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
|
||||
|
||||
|
||||
class TestScopedToNamespacedKeys:
|
||||
def test_bare_state_key_is_not_rewritten(self, plugins_dir, tmp_path):
|
||||
"""ledmatrix-elections' ``state`` is a two-letter code, not a place name."""
|
||||
write_plugin(plugins_dir, "ledmatrix-elections", {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"state": {"type": "string", "default": "CA"},
|
||||
"city": {"type": "string", "default": "Springfield"},
|
||||
},
|
||||
})
|
||||
sm, _ = make_sm(plugins_dir, tmp_path,
|
||||
{"city": "Kansas City", "state": "Missouri", "country": "US"})
|
||||
|
||||
defaults = sm.generate_default_config("ledmatrix-elections")
|
||||
|
||||
assert defaults["state"] == "CA"
|
||||
assert defaults["city"] == "Springfield"
|
||||
|
||||
def test_plugin_without_location_fields_never_reads_config(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "clock-simple", {
|
||||
"type": "object",
|
||||
"properties": {"format": {"type": "string", "default": "12h"}},
|
||||
})
|
||||
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
|
||||
|
||||
defaults = sm.generate_default_config("clock-simple")
|
||||
|
||||
assert defaults["format"] == "12h"
|
||||
assert cm.load_count == 0
|
||||
|
||||
|
||||
class TestCachingStaysFresh:
|
||||
def test_location_change_is_picked_up_through_the_defaults_cache(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
|
||||
|
||||
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Kansas City"
|
||||
|
||||
cm.config["location"]["city"] = "Omaha"
|
||||
|
||||
# Second call is served from the defaults cache, but must not serve a
|
||||
# stale location.
|
||||
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Omaha"
|
||||
|
||||
def test_cached_defaults_are_not_mutated_by_the_overlay(self, plugins_dir, tmp_path):
|
||||
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
|
||||
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
|
||||
|
||||
sm.generate_default_config("ledmatrix-weather")
|
||||
assert sm._defaults_cache["ledmatrix-weather"]["location_city"] == "Dallas"
|
||||
|
||||
cm.config.pop("location")
|
||||
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
|
||||
@@ -0,0 +1,147 @@
|
||||
"""The captive portal's fixed-argument sudo calls must be granted.
|
||||
|
||||
The installers write two allow-lists, /etc/sudoers.d/ledmatrix_web and
|
||||
ledmatrix_wifi. A sudo call absent from both needs a password, which a service
|
||||
cannot supply, so it fails.
|
||||
|
||||
Four such calls were ungranted, all of them captive-portal teardown/setup:
|
||||
|
||||
sysctl -w net.ipv4.ip_forward=0|1 wifi_manager.py:788, 883
|
||||
nft add|delete table ip ledmatrix wifi_manager.py:835, 895
|
||||
rfkill unblock wifi wifi_manager.py:1811
|
||||
mkdir -p .../dnsmasq-shared.d wifi_manager.py:922
|
||||
|
||||
It goes unnoticed because a stock Raspberry Pi image ships
|
||||
/etc/sudoers.d/010_pi-nopasswd granting the default user
|
||||
`ALL=(ALL) NOPASSWD: ALL`, which satisfies every gap in both files. It only
|
||||
bites once that blanket rule is removed or the service runs as another user.
|
||||
|
||||
Scope, deliberately narrow: this pins the four commands above, each of which
|
||||
can be written out literally. The portal makes further sudo calls whose
|
||||
arguments are built at runtime -- iptables and nft rules carrying an interface
|
||||
name and a port, `ip addr`, `ip link` -- and those cannot be granted safely
|
||||
here. A rule covering them needs a trailing wildcard, and
|
||||
`iptables --modprobe=/path/to/anything` runs that path as root, so
|
||||
`NOPASSWD: iptables *` is a root shell for the web user by another name.
|
||||
Closing that half needs a privileged helper that builds the rules itself and
|
||||
takes only an interface and a port, granted the way safe_plugin_rm.sh already
|
||||
is. That is a design decision, not a one-line grant, and belongs in its own
|
||||
change.
|
||||
"""
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALLERS = (
|
||||
ROOT / "first_time_install.sh",
|
||||
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
|
||||
)
|
||||
|
||||
#: Commands this change grants, each fully literal in the source.
|
||||
REQUIRED = (
|
||||
("sysctl", "-w", "net.ipv4.ip_forward=0"),
|
||||
("sysctl", "-w", "net.ipv4.ip_forward=1"),
|
||||
("nft", "add", "table", "ip", "ledmatrix"),
|
||||
("nft", "delete", "table", "ip", "ledmatrix"),
|
||||
("rfkill", "unblock", "wifi"),
|
||||
("mkdir", "-p", "/etc/NetworkManager/dnsmasq-shared.d"),
|
||||
)
|
||||
|
||||
#: Tools with an option that executes a program of the caller's choosing.
|
||||
#: A trailing wildcard on any of these is a privilege escalation.
|
||||
EXEC_CAPABLE = ("iptables", "ip6tables", "nft", "tcpdump", "find", "awk",
|
||||
"sed", "perl", "python", "python3", "env")
|
||||
|
||||
|
||||
def _grant_lines():
|
||||
lines = []
|
||||
for installer in INSTALLERS:
|
||||
if not installer.is_file():
|
||||
continue
|
||||
for line in installer.read_text(encoding="utf-8", errors="replace").splitlines():
|
||||
if "NOPASSWD:" in line:
|
||||
lines.append(line.split("NOPASSWD:", 1)[1])
|
||||
return lines
|
||||
|
||||
|
||||
def _normalise(rule):
|
||||
"""One rule with binary-path variables reduced to bare tool names.
|
||||
|
||||
Rules are written as `$SYSCTL_PATH -w ...` or `${NFT_PATH} ...`, so
|
||||
matching the literal "sysctl" finds nothing and every rule looks absent --
|
||||
which is exactly how an earlier version of this test reported six gaps
|
||||
that did not exist. Both spellings are handled: shell expands them
|
||||
identically, and a check that understood only one silently skipped the
|
||||
other.
|
||||
"""
|
||||
rule = re.sub(r"\$\{?([A-Z][A-Z0-9_]*)_PATH\}?",
|
||||
lambda m: m.group(1).lower(), rule)
|
||||
return re.sub(r"/usr/(?:s?bin)/", "", rule)
|
||||
|
||||
|
||||
def _granted_commands():
|
||||
"""The command each NOPASSWD rule actually grants, normalised.
|
||||
|
||||
_grant_lines() already returns everything after "NOPASSWD:", so what
|
||||
arrives here is the command, possibly preceded by the NOEXEC tag and
|
||||
possibly still carrying the closing quote of an `echo "..."` that wrote
|
||||
it. Both are stripped so the result is comparable to a plain command.
|
||||
"""
|
||||
commands = []
|
||||
for rule in _grant_lines():
|
||||
command = _normalise(rule).strip()
|
||||
command = re.sub(r"^NOEXEC:\s*", "", command)
|
||||
command = command.rstrip('"').rstrip("'").strip()
|
||||
if command:
|
||||
commands.append(" ".join(command.split()))
|
||||
return commands
|
||||
|
||||
|
||||
def test_the_installers_are_present():
|
||||
missing = [str(p.relative_to(ROOT)) for p in INSTALLERS if not p.is_file()]
|
||||
assert not missing, f"installer(s) missing: {missing}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("command", REQUIRED, ids=lambda c: " ".join(c))
|
||||
def test_the_command_is_granted(command):
|
||||
"""Whole command, not just the binary.
|
||||
|
||||
Checking only the binary made this far weaker than it looked: with
|
||||
`sysctl` present anywhere, deleting the ip_forward=0 grant still passed,
|
||||
and the portal would then be unable to restore forwarding on teardown.
|
||||
"""
|
||||
wanted = " ".join(command)
|
||||
granted = _granted_commands()
|
||||
# Exact match, not a prefix. A substring search was satisfied by
|
||||
# `sysctl -w net.ipv4.ip_forward=0 *`, and that trailing wildcard lets the
|
||||
# caller append whatever they like to a command running as root -- a far
|
||||
# wider grant than the one this test is meant to be confirming.
|
||||
assert wanted in granted, (
|
||||
f"no installer grants exactly `{wanted}`; closest matches: "
|
||||
+ str([g for g in granted if g.startswith(command[0])])[:200])
|
||||
|
||||
|
||||
def test_no_wildcard_on_a_tool_that_can_exec():
|
||||
"""`NOPASSWD: iptables *` hands the web user root.
|
||||
|
||||
iptables --modprobe=/path runs that path as root. This caught a grant added
|
||||
in this very change, which is why it is here.
|
||||
"""
|
||||
offenders = []
|
||||
for rule in _grant_lines():
|
||||
rule = rule.strip()
|
||||
if not rule.endswith("*"):
|
||||
continue
|
||||
# Normalised the same way as everything else: `${NFT_PATH} *` left a
|
||||
# brace before the tool name, and the word-boundary check below does
|
||||
# not treat "{" as a boundary, so that spelling slipped through.
|
||||
haystack = _normalise(rule).lower()
|
||||
for tool in EXEC_CAPABLE:
|
||||
if re.search(rf"(^|/|\s|\$){tool}(\s|$)", haystack):
|
||||
offenders.append(rule)
|
||||
break
|
||||
assert not offenders, (
|
||||
"wildcard grant on a tool that can execute another program:\n "
|
||||
+ "\n ".join(offenders))
|
||||
@@ -30,6 +30,10 @@ ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALLERS = (
|
||||
ROOT / "first_time_install.sh",
|
||||
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
|
||||
# Writes the same journalctl grants as first_time_install.sh. It was
|
||||
# missing here, and because of that this suite passed while three
|
||||
# ungranted wildcard rules sat in it.
|
||||
ROOT / "scripts" / "install" / "configure_web_sudo.sh",
|
||||
)
|
||||
|
||||
#: Commands that will start another program of their own accord -- a pager, an
|
||||
@@ -44,8 +48,14 @@ def _grant_lines():
|
||||
continue
|
||||
for line in installer.read_text(encoding="utf-8", errors="replace").splitlines():
|
||||
stripped = line.strip()
|
||||
if "NOPASSWD" in stripped and not stripped.startswith("#"):
|
||||
lines.append(stripped)
|
||||
if "NOPASSWD" not in stripped or stripped.startswith("#"):
|
||||
continue
|
||||
# Installers emit rules two ways: written literally into a heredoc,
|
||||
# or echoed into a file. An echoed rule ends in a quote, so the
|
||||
# trailing-wildcard check below would skip it and the rule would
|
||||
# never be examined at all.
|
||||
echoed = re.fullmatch(r"""echo\s+(['"])(.*)\1""", stripped)
|
||||
lines.append(echoed.group(2) if echoed else stripped)
|
||||
return lines
|
||||
|
||||
|
||||
@@ -78,10 +88,12 @@ def test_journalctl_is_granted_at_all():
|
||||
"no journalctl grant remains; the web interface reads logs through it")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("unit", ["ledmatrix.service", "ledmatrix"])
|
||||
def test_each_journalctl_rule_is_tagged(unit):
|
||||
@pytest.mark.parametrize("selector", ["-u ledmatrix.service", "-u ledmatrix",
|
||||
"-t ledmatrix"])
|
||||
def test_each_journalctl_rule_is_tagged(selector):
|
||||
"""Every selector, so removing one cannot pass by the others' presence."""
|
||||
matching = [r for r in _grant_lines()
|
||||
if "JOURNALCTL_PATH" in r and f"-u {unit} " in r]
|
||||
assert matching, f"no journalctl rule for -u {unit}"
|
||||
if "JOURNALCTL_PATH" in r and f"{selector} " in r]
|
||||
assert matching, f"no journalctl rule for {selector}"
|
||||
untagged = [r for r in matching if "NOEXEC" not in r]
|
||||
assert not untagged, f"untagged journalctl rule(s): {untagged}"
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
"""The display unit must cap glibc's malloc arenas.
|
||||
|
||||
glibc hands each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||
and an arena that has grown is never returned to the OS. This process runs
|
||||
threads for the render loop, the update workers and the background fetchers, so
|
||||
on a 3-core Pi the ceiling is 24 arenas.
|
||||
|
||||
Measured on a live rig, 2.5 hours in:
|
||||
|
||||
RSS 1030 MB
|
||||
Private_Dirty 988 MB
|
||||
anonymous mappings > 10 MB 23 (ceiling is 8 x 3 = 24)
|
||||
largest few 104, 79, 66, 63, 63 MB, on 64 MB-aligned addresses
|
||||
|
||||
against live data that accounts for perhaps 15 MB -- the widest scroll strip
|
||||
observed was 35,746 x 64, about 7 MB as RGB and the same again for its numpy
|
||||
mirror. Repeated sampling showed RSS flat between 990 and 1030 MB rather than
|
||||
climbing, so this is arena bloat rather than a leak: memory Python has freed
|
||||
but glibc is holding per-arena.
|
||||
|
||||
The device had 59 MB free at the time.
|
||||
|
||||
Capping the arena count trades a little allocator concurrency for that resident
|
||||
memory. The render loop is latency-sensitive, so if p99 frame time regresses the
|
||||
right response is to raise this rather than remove it.
|
||||
"""
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
UNIT = (Path(__file__).resolve().parent.parent / "systemd" / "ledmatrix.service")
|
||||
|
||||
#: The value the unit is expected to carry. 2 is the usual choice for a
|
||||
#: threaded Python process; 1-4 all keep some of the saving, but only one of
|
||||
#: them is what this project ships.
|
||||
EXPECTED_ARENA_MAX = 2
|
||||
|
||||
|
||||
def _environment(unit_text):
|
||||
return dict(
|
||||
line.split("=", 2)[1:3] if line.count("=") >= 2 else (line.split("=", 1)[1], "")
|
||||
for line in unit_text.splitlines()
|
||||
if line.startswith("Environment=")
|
||||
)
|
||||
|
||||
|
||||
def test_the_unit_exists():
|
||||
assert UNIT.is_file(), f"{UNIT} is missing"
|
||||
|
||||
|
||||
def test_malloc_arena_max_is_capped():
|
||||
env = _environment(UNIT.read_text(encoding="utf-8"))
|
||||
assert "MALLOC_ARENA_MAX" in env, (
|
||||
"the display unit does not cap glibc arenas; on a 3-core Pi the default "
|
||||
"ceiling is 24 and a measured rig held 23 of them, 920 MB"
|
||||
)
|
||||
value = int(env["MALLOC_ARENA_MAX"])
|
||||
# Pinned, not a range. A range let a change to 4 -- which hands most of the
|
||||
# saving back -- pass unnoticed, which was the point of the finding that
|
||||
# prompted this. Raising it is a legitimate response to a frame-time
|
||||
# regression, but it should be a visible edit here rather than a silent
|
||||
# drift, so the number lives in one place and changing it shows up in
|
||||
# review.
|
||||
assert value == EXPECTED_ARENA_MAX, (
|
||||
f"MALLOC_ARENA_MAX={value}, expected {EXPECTED_ARENA_MAX}. If this was "
|
||||
"raised deliberately because frame times regressed, update "
|
||||
"EXPECTED_ARENA_MAX here and say so in the commit."
|
||||
)
|
||||
|
||||
|
||||
def test_the_reason_is_recorded_next_to_it():
|
||||
"""A bare tuning knob invites removal by whoever meets it next."""
|
||||
text = UNIT.read_text(encoding="utf-8")
|
||||
index = text.index("Environment=MALLOC_ARENA_MAX")
|
||||
preamble = text[:index].splitlines()[-12:]
|
||||
comment = "\n".join(line for line in preamble if line.startswith("#"))
|
||||
assert "arena" in comment.lower(), "no explanation precedes the setting"
|
||||
assert re.search(r"\d", comment), (
|
||||
"the explanation cites no measurement, so a reader cannot tell whether "
|
||||
"it still applies to their hardware"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("unit", ["ledmatrix.service"])
|
||||
def test_the_unit_still_parses_as_ini(unit):
|
||||
"""systemd will refuse a malformed unit, and the panel stays dark."""
|
||||
import configparser
|
||||
|
||||
path = UNIT.parent / unit
|
||||
parser = configparser.ConfigParser(strict=False)
|
||||
# systemd allows repeated keys; ConfigParser needs them merged, not rejected.
|
||||
parser.read_string(path.read_text(encoding="utf-8"))
|
||||
assert parser.has_section("Service")
|
||||
assert parser.has_option("Service", "ExecStart")
|
||||
@@ -0,0 +1,152 @@
|
||||
"""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_drift():
|
||||
"""Order is not noise in a systemd unit.
|
||||
|
||||
Repeated directives -- ExecStartPre=, ExecStartPost= -- run in the order
|
||||
they appear, and a directive that moves between [Unit], [Service] and
|
||||
[Install] means something different, or nothing, where it lands. This
|
||||
check used to sort the lines before comparing, which reported no drift for
|
||||
a unit that had genuinely changed.
|
||||
"""
|
||||
a = "[Service]\nExecStartPre=/first\nExecStartPre=/second\n"
|
||||
b = "[Service]\nExecStartPre=/second\nExecStartPre=/first\n"
|
||||
assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b), (
|
||||
"swapping two ExecStartPre= lines changes what runs first, and was "
|
||||
"being normalised away")
|
||||
|
||||
|
||||
def test_a_directive_moved_between_sections_is_drift():
|
||||
a = "[Unit]\nDescription=x\n[Service]\nExecStart=/x\n"
|
||||
b = "[Unit]\nDescription=x\nExecStart=/x\n[Service]\n"
|
||||
assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b), (
|
||||
"ExecStart= in [Unit] is not the same unit, and sorting hid it")
|
||||
|
||||
|
||||
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"))
|
||||
# Cosmetic means comments, blank lines and stray indentation -- the things
|
||||
# the installer really does drop. Not reordering: that changes the unit,
|
||||
# and is asserted as drift above.
|
||||
directives = [line.strip() for line in substituted.splitlines()
|
||||
if line.strip() and not line.strip().startswith("#")]
|
||||
installed = tmp_path / "ledmatrix.service"
|
||||
installed.write_text(
|
||||
"\n\n".join(" " + d for d in 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
|
||||
@@ -0,0 +1,85 @@
|
||||
"""A check that could not run must not be reported as "up to date".
|
||||
|
||||
check-update returned update_available=False whenever git failed. The banner
|
||||
is the only route to the update button, so a checkout git refuses to touch
|
||||
looked exactly like a current one -- permanently, and with nothing for the
|
||||
user to act on. The usual cause is an install performed as root, after which
|
||||
every git command fails with "detected dubious ownership".
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod # noqa: E402
|
||||
from web_interface.blueprints.api_v3 import api_v3 # noqa: E402
|
||||
|
||||
DUBIOUS = ("fatal: detected dubious ownership in repository at "
|
||||
"'/home/pi/LEDMatrix'\nTo add an exception for this directory, call:\n"
|
||||
"\tgit config --global --add safe.directory /home/pi/LEDMatrix\n")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
app = Flask(__name__)
|
||||
app.config['TESTING'] = True
|
||||
app.register_blueprint(api_v3, url_prefix='/api/v3')
|
||||
mod._update_check_cache['result'] = None
|
||||
mod._update_check_cache['ts'] = 0
|
||||
return app.test_client()
|
||||
|
||||
|
||||
def _fetch_fails(stderr: bytes):
|
||||
def fake_run(args, **kwargs):
|
||||
if args[:2] == ['git', 'fetch']:
|
||||
return subprocess.CompletedProcess(args, 1, stdout=b'', stderr=stderr)
|
||||
return subprocess.CompletedProcess(args, 0, stdout='', stderr='')
|
||||
return fake_run
|
||||
|
||||
|
||||
class TestFailedCheckIsNotSilence:
|
||||
def test_dubious_ownership_is_reported_not_swallowed(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['check_failed'] is True, (
|
||||
"a git failure was reported as a successful 'no update' check")
|
||||
assert data['update_available'] is False
|
||||
|
||||
def test_the_message_tells_the_user_what_to_do(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert 'chown' in data['error'], (
|
||||
"dubious ownership is unactionable without the fix command")
|
||||
assert 'root' in data['error']
|
||||
|
||||
def test_an_ordinary_git_failure_still_surfaces(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_fetch_fails(b'fatal: some other git problem\n')):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['check_failed'] is True
|
||||
assert 'some other git problem' in data['error']
|
||||
|
||||
def test_offline_reads_as_offline(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_fetch_fails(b'fatal: could not resolve host: github.com\n')):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert 'Could not reach GitHub' in data['error']
|
||||
|
||||
|
||||
class TestSuccessPathUnchanged:
|
||||
def test_up_to_date_carries_no_failure_flag(self, client):
|
||||
def fake_run(args, **kwargs):
|
||||
if args[:2] == ['git', 'fetch']:
|
||||
return subprocess.CompletedProcess(args, 0, stdout=b'', stderr=b'')
|
||||
if args[:2] == ['git', 'rev-parse']:
|
||||
return subprocess.CompletedProcess(args, 0, stdout='abc123\n', stderr='')
|
||||
return subprocess.CompletedProcess(args, 0, stdout='0\n', stderr='')
|
||||
with patch.object(mod.subprocess, 'run', fake_run):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['update_available'] is False
|
||||
assert not data.get('check_failed'), "a healthy check must not look like a failure"
|
||||
@@ -0,0 +1,84 @@
|
||||
"""A pull that changed nothing on the running system is not an applied update.
|
||||
|
||||
git_pull replaces files on disk and restarts nothing -- there is no systemctl
|
||||
call anywhere in the handler. The display and web services keep running the
|
||||
code they loaded at boot, so the user is told "Code updated successfully" and
|
||||
sees no change until they happen to reboot. The response now says whether a
|
||||
restart is owed, and the UI raises the existing restart-pending banner.
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod # noqa: E402
|
||||
from web_interface.blueprints.api_v3 import api_v3 # noqa: E402
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
app = Flask(__name__)
|
||||
app.config['TESTING'] = True
|
||||
app.register_blueprint(api_v3, url_prefix='/api/v3')
|
||||
# The handler consults these after a successful pull; None is the
|
||||
# "not wired up" case it already guards for.
|
||||
api_v3.plugin_store_manager = None
|
||||
api_v3.config_manager = None
|
||||
return app.test_client()
|
||||
|
||||
|
||||
def _git(heads, pull_rc=0, pull_out='Updating a1b2c3..d4e5f6\n'):
|
||||
"""Fake git. `heads` are the successive answers to rev-parse HEAD."""
|
||||
seq = list(heads)
|
||||
|
||||
def run(args, **kwargs):
|
||||
def ok(stdout='', rc=0, b=False):
|
||||
return subprocess.CompletedProcess(
|
||||
args, rc, stdout=(stdout.encode() if b else stdout),
|
||||
stderr=(b'' if b else ''))
|
||||
if args[:2] == ['git', 'rev-parse'] and args[-1] == 'HEAD':
|
||||
return ok(seq.pop(0) + '\n' if seq else 'deadbeef\n')
|
||||
if 'symbolic-full-name' in args or '@{u}' in args:
|
||||
return ok('origin/main\n')
|
||||
if args[:2] == ['git', 'status']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'diff']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'pull']:
|
||||
return ok(pull_out, pull_rc)
|
||||
return ok('')
|
||||
return run
|
||||
|
||||
|
||||
def _pull(client):
|
||||
return client.post('/api/v3/system/action',
|
||||
json={'action': 'git_pull'}).get_json()
|
||||
|
||||
|
||||
class TestRestartIsRequestedWhenCodeChanged:
|
||||
def test_a_pull_that_moved_head_asks_for_a_restart(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is True, (
|
||||
"new code on disk, services still running the old code, and "
|
||||
"nothing told the user to restart")
|
||||
|
||||
def test_already_up_to_date_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is False, (
|
||||
"prompting after a no-op update trains users to ignore the prompt")
|
||||
|
||||
def test_a_failed_pull_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'error'
|
||||
assert data['restart_required'] is False
|
||||
@@ -194,15 +194,46 @@ class TestSavePluginConfig:
|
||||
|
||||
def test_secret_count_message_counts_top_level_keys(self, env):
|
||||
# Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
|
||||
# the separated secrets dict. Here that is 2: the posted accounts
|
||||
# array (all its item tokens count as ONE key) plus the schema's
|
||||
# api_key default ("") that merge_with_defaults adds before
|
||||
# separation.
|
||||
# the separated secrets dict. Here that is 1: the posted accounts
|
||||
# array, whose item tokens all count as ONE key.
|
||||
#
|
||||
# It was 2 before blank secrets were dropped, the second being the
|
||||
# schema's api_key default (""), which merge_with_defaults adds to
|
||||
# every save. Counting it was the visible edge of a real bug: that
|
||||
# injected blank was merged over the stored api_key, so saving any
|
||||
# unrelated field destroyed the credential. See
|
||||
# test_an_unrelated_edit_does_not_erase_a_stored_secret.
|
||||
resp = self._save(env, {
|
||||
"accounts": [{"name": "a", "token": "t"}],
|
||||
})
|
||||
message = resp.get_json()["message"]
|
||||
assert "(2 secret field(s) saved to config_secrets.json)" in message
|
||||
assert "(1 secret field(s) saved to config_secrets.json)" in message
|
||||
|
||||
def test_an_unrelated_edit_does_not_erase_a_stored_secret(self, env):
|
||||
"""Editing one field must not wipe the plugin's API key.
|
||||
|
||||
The config form renders secrets masked, so the browser posts them
|
||||
back blank; merge_with_defaults injects a blank api_key even when
|
||||
the client omits it entirely. Either way a "" reached the secrets
|
||||
file and deep_merge wrote it over the stored credential.
|
||||
"""
|
||||
assert self._save(env, {"api_key": "REAL-KEY-0123456789",
|
||||
"city": "Austin"}).status_code == 200
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
||||
"REAL-KEY-0123456789"
|
||||
|
||||
# the user changes the city; the masked api_key rides along blank
|
||||
assert self._save(env, {"api_key": "", "city": "Dallas"}).status_code == 200
|
||||
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
||||
"REAL-KEY-0123456789", "an unrelated edit destroyed the API key"
|
||||
assert env.fresh_load()[PLUGIN_ID]["city"] == "Dallas"
|
||||
|
||||
def test_a_secret_can_still_be_changed(self, env):
|
||||
"""Dropping blanks must not stop a real new value from being saved."""
|
||||
self._save(env, {"api_key": "first-key"})
|
||||
self._save(env, {"api_key": "second-key"})
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == "second-key"
|
||||
|
||||
def test_resave_replaces_stored_secrets_list_wholesale(self, env):
|
||||
# Characterized: api_v3's deep_merge intentionally replaces lists,
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
"""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)
|
||||
# Assert the write succeeded. A 500 leaves the old file in place, so the
|
||||
# assertions below would hold without the write path running at all.
|
||||
resp = env.client.post("/api/v3/config/raw/secrets", json=_get(env))
|
||||
assert resp.status_code == 200, resp.get_data(as_text=True)[:200]
|
||||
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
|
||||
|
||||
|
||||
def test_a_list_of_secrets_keeps_its_shape(env):
|
||||
"""A list must not be masked as though it were one scalar.
|
||||
|
||||
accounts: [{...}, {...}] came back as a single '••••••••', so a caller
|
||||
could not see how many entries existed, and the raw editor was shown a
|
||||
string where the file holds an array.
|
||||
"""
|
||||
env.secrets_file.write_text(json.dumps({
|
||||
"myplugin": {"accounts": [{"name": "a", "token": "tok-a"},
|
||||
{"name": "b", "token": "tok-b"}]}}))
|
||||
accounts = _get(env)["myplugin"]["accounts"]
|
||||
assert isinstance(accounts, list), "the list was flattened to a scalar"
|
||||
assert len(accounts) == 2, "entries were lost"
|
||||
assert all(isinstance(a, dict) for a in accounts), "entry shape was lost"
|
||||
assert "tok-a" not in json.dumps(accounts), "a token survived masking"
|
||||
|
||||
|
||||
def test_a_list_posted_back_unchanged_is_left_alone(env):
|
||||
"""Lists merge by replacement, so a half-masked list must not be stored."""
|
||||
original = {"myplugin": {"accounts": [{"name": "a", "token": "tok-a"},
|
||||
{"name": "b", "token": "tok-b"}]}}
|
||||
env.secrets_file.write_text(json.dumps(original))
|
||||
resp = env.client.post("/api/v3/config/raw/secrets", json=_get(env))
|
||||
assert resp.status_code == 200, resp.get_data(as_text=True)[:200]
|
||||
assert _on_disk(env.secrets_file)["myplugin"]["accounts"] == \
|
||||
original["myplugin"]["accounts"], "round-tripping the mask damaged the list"
|
||||
|
||||
|
||||
def test_a_fully_supplied_list_still_saves(env):
|
||||
env.secrets_file.write_text(json.dumps(
|
||||
{"myplugin": {"accounts": [{"name": "a", "token": "old"}]}}))
|
||||
body = _get(env)
|
||||
body["myplugin"]["accounts"] = [{"name": "a", "token": "new"}]
|
||||
resp = env.client.post("/api/v3/config/raw/secrets", json=body)
|
||||
assert resp.status_code == 200, resp.get_data(as_text=True)[:200]
|
||||
assert _on_disk(env.secrets_file)["myplugin"]["accounts"][0]["token"] == "new"
|
||||
@@ -118,8 +118,7 @@ saved_repositories_manager = SavedRepositoriesManager()
|
||||
schema_manager = SchemaManager(
|
||||
plugins_dir=plugins_dir,
|
||||
project_root=project_root,
|
||||
logger=None,
|
||||
config_manager=config_manager
|
||||
logger=None
|
||||
)
|
||||
|
||||
# Initialize operation queue for plugin operations
|
||||
|
||||
@@ -21,7 +21,9 @@ 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,
|
||||
remove_empty_secrets, 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 (
|
||||
@@ -262,15 +264,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
|
||||
@@ -715,10 +756,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()
|
||||
@@ -1216,6 +1259,11 @@ def save_main_config():
|
||||
|
||||
# Separate secrets from regular config (same logic as save_plugin_config)
|
||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||
# The config form renders secrets masked, so every save posts
|
||||
# them back blank. Without this the blank is merged over the
|
||||
# stored value and the credential is destroyed by the act of
|
||||
# changing an unrelated setting. A blank means "unchanged".
|
||||
secrets_config = remove_empty_secrets(secrets_config)
|
||||
|
||||
# PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
|
||||
# This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
|
||||
@@ -1333,7 +1381,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
|
||||
@@ -1401,8 +1454,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:
|
||||
@@ -1660,13 +1724,22 @@ def resolve_pull_command(project_dir):
|
||||
backup, or following an install guide that names one. The update button
|
||||
then reports a failure the user cannot act on.
|
||||
|
||||
``--autostash`` is passed for the same reason. Rebase refuses to start
|
||||
when any tracked file is modified, and on these installs something always
|
||||
is: first_time_install.sh chmods five scripts that git tracked as 644, so
|
||||
every machine that ran the installer carries five permanent mode changes
|
||||
and the update button reports "cannot pull with rebase: You have unstaged
|
||||
changes". Those modes are corrected in this commit, but a user cannot pull
|
||||
the correction while the pull is what is blocked, and any other local edit
|
||||
would reproduce it anyway. Autostash reapplies the changes afterwards.
|
||||
|
||||
Returns ``(args, note, error)``. When ``origin/<branch>`` exists the pull
|
||||
is made explicit against it, so the update proceeds and the branch is
|
||||
given tracking information afterwards.
|
||||
"""
|
||||
upstream = _git_upstream(project_dir)
|
||||
if upstream:
|
||||
return ['git', 'pull', '--rebase'], '', None
|
||||
return ['git', 'pull', '--rebase', '--autostash'], '', None
|
||||
|
||||
branch = _git_current_branch(project_dir)
|
||||
if not branch:
|
||||
@@ -1676,7 +1749,7 @@ def resolve_pull_command(project_dir):
|
||||
)
|
||||
if _git_remote_branch_exists(project_dir, branch):
|
||||
return (
|
||||
['git', 'pull', '--rebase', 'origin', branch],
|
||||
['git', 'pull', '--rebase', '--autostash', 'origin', branch],
|
||||
f"Branch '{branch}' had no upstream; pulled from origin/{branch} and set it as the upstream.",
|
||||
None,
|
||||
)
|
||||
@@ -1824,6 +1897,33 @@ def get_system_version():
|
||||
_update_check_cache: Dict[str, Any] = {'result': None, 'ts': 0.0}
|
||||
_UPDATE_CHECK_TTL = 300 # 5 minutes — avoids a git fetch on every page load
|
||||
|
||||
def _update_check_failed(detail: str) -> Dict[str, Any]:
|
||||
"""A check that could not run is not the same as being up to date.
|
||||
|
||||
Reporting update_available=False on a git failure hides the banner, and
|
||||
the banner is the only route to the update button -- so a checkout git
|
||||
refuses to touch looks exactly like a current one, permanently. The most
|
||||
common cause is an install performed as root: git then reports "dubious
|
||||
ownership" and every command fails, including the fetch here.
|
||||
"""
|
||||
return {'update_available': False, 'remote_sha': 'unknown',
|
||||
'commits_behind': 0, 'check_failed': True, 'error': detail}
|
||||
|
||||
|
||||
def _describe_git_failure(stderr: str) -> str:
|
||||
"""Turn git's stderr into something the user can act on."""
|
||||
text = (stderr or '').strip()
|
||||
if 'dubious ownership' in text or 'detected dubious ownership' in text:
|
||||
return ("This checkout is owned by a different user than the one "
|
||||
"running the web interface, so git refuses to use it. It is "
|
||||
"usually the result of installing as root. Fix the ownership "
|
||||
"and the update will work: sudo chown -R $USER:$USER "
|
||||
+ str(PROJECT_ROOT))
|
||||
if 'could not resolve host' in text.lower() or 'network is unreachable' in text.lower():
|
||||
return "Could not reach GitHub to check for updates."
|
||||
return "Could not check for updates: " + (text.splitlines()[0] if text else "git failed")
|
||||
|
||||
|
||||
@api_v3.route('/system/check-update', methods=['GET'])
|
||||
def check_for_update():
|
||||
"""Check whether a newer LEDMatrix commit is available on origin/main."""
|
||||
@@ -1839,12 +1939,13 @@ def check_for_update():
|
||||
capture_output=True, timeout=10, cwd=cwd,
|
||||
)
|
||||
if fetch_result.returncode != 0:
|
||||
stderr = fetch_result.stderr.decode(errors='replace').strip()
|
||||
logger.warning("check-update: git fetch failed (rc=%d): %s",
|
||||
fetch_result.returncode,
|
||||
fetch_result.stderr.decode(errors='replace').strip())
|
||||
_update_check_cache['result'] = _safe
|
||||
fetch_result.returncode, stderr)
|
||||
failed = _update_check_failed(_describe_git_failure(stderr))
|
||||
_update_check_cache['result'] = failed
|
||||
_update_check_cache['ts'] = now
|
||||
return jsonify(_safe)
|
||||
return jsonify(failed)
|
||||
local = subprocess.run(
|
||||
['git', 'rev-parse', 'HEAD'],
|
||||
capture_output=True, text=True, timeout=5, cwd=cwd,
|
||||
@@ -1872,7 +1973,8 @@ def check_for_update():
|
||||
return jsonify(result)
|
||||
except Exception as e:
|
||||
logger.warning("check-update failed: %s", e)
|
||||
return jsonify(_safe)
|
||||
return jsonify(_update_check_failed(
|
||||
"Could not check for updates; see logs for details."))
|
||||
|
||||
@api_v3.route('/system/action', methods=['POST'])
|
||||
def execute_system_action():
|
||||
@@ -1999,6 +2101,11 @@ def execute_system_action():
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning("git rev-parse timed out before pull")
|
||||
|
||||
# Whether the pull actually brought new code in. "Already up to
|
||||
# date" is a success too, and prompting for a restart then would
|
||||
# train users to ignore the prompt.
|
||||
code_changed = False
|
||||
|
||||
# Perform the git pull. Branches without an upstream were given
|
||||
# an explicit "origin <branch>" above so the update still works.
|
||||
result = subprocess.run(
|
||||
@@ -2042,6 +2149,7 @@ def execute_system_action():
|
||||
capture_output=True, text=True, timeout=10, cwd=project_dir)
|
||||
new_head = _post.stdout.strip() if _post.returncode == 0 else None
|
||||
if old_head and new_head and old_head != new_head:
|
||||
code_changed = True
|
||||
diff = subprocess.run(
|
||||
['git', 'diff', '--name-only', f'{old_head}..{new_head}'],
|
||||
capture_output=True, text=True, timeout=15, cwd=project_dir)
|
||||
@@ -2101,9 +2209,14 @@ def execute_system_action():
|
||||
if ln.strip()), '')
|
||||
pull_message = f"Update failed: {detail}" if detail else "Update failed; check logs for details"
|
||||
|
||||
# Nothing here restarts anything: the pull replaces files on
|
||||
# disk while the display and web services keep running the code
|
||||
# they loaded at boot. Without this the user is told the update
|
||||
# succeeded and sees no change until they happen to reboot.
|
||||
return jsonify({
|
||||
'status': 'success' if result.returncode == 0 else 'error',
|
||||
'message': pull_message,
|
||||
'restart_required': bool(result.returncode == 0 and code_changed),
|
||||
})
|
||||
elif action == 'checkout_branch':
|
||||
# Switch branches from the Tools tab. Needed because a checkout
|
||||
@@ -5617,6 +5730,11 @@ def save_plugin_config():
|
||||
# Separate secrets from regular config (handles nested configs and
|
||||
# array-item secrets — see src/web_interface/secret_helpers.py)
|
||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||
# The config form renders secrets masked, so every save posts
|
||||
# them back blank. Without this the blank is merged over the
|
||||
# stored value and the credential is destroyed by the act of
|
||||
# changing an unrelated setting. A blank means "unchanged".
|
||||
secrets_config = remove_empty_secrets(secrets_config)
|
||||
|
||||
# Get current configs
|
||||
current_config = api_v3.config_manager.load_config()
|
||||
|
||||
@@ -116,14 +116,25 @@ document.body.addEventListener('htmx:afterRequest', function(event) {
|
||||
// ===== Restart-pending banner =====
|
||||
// Shown after restart-requiring saves; persists across tab switches (and
|
||||
// reloads, via sessionStorage) until the display restarts or it's dismissed.
|
||||
window.showRestartPending = function() {
|
||||
try { sessionStorage.setItem('ledmatrix-restart-pending', '1'); } catch { /* private browsing */ }
|
||||
window.showRestartPending = function(message) {
|
||||
try {
|
||||
sessionStorage.setItem('ledmatrix-restart-pending', '1');
|
||||
// Persisted alongside the flag: a code update and a config save want
|
||||
// different wording, and the banner outlives the page that raised it.
|
||||
if (message) sessionStorage.setItem('ledmatrix-restart-pending-text', message);
|
||||
else sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* private browsing */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && message) text.textContent = message;
|
||||
if (banner) banner.style.display = 'block';
|
||||
};
|
||||
|
||||
window.dismissRestartPending = function() {
|
||||
try { sessionStorage.removeItem('ledmatrix-restart-pending'); } catch { /* no-op */ }
|
||||
try {
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending');
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* no-op */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
if (banner) banner.style.display = 'none';
|
||||
};
|
||||
@@ -151,6 +162,9 @@ document.addEventListener('DOMContentLoaded', function() {
|
||||
try {
|
||||
if (sessionStorage.getItem('ledmatrix-restart-pending') === '1') {
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const saved = sessionStorage.getItem('ledmatrix-restart-pending-text');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && saved) text.textContent = saved;
|
||||
if (banner) banner.style.display = 'block';
|
||||
}
|
||||
} catch { /* no-op */ }
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -413,7 +413,8 @@
|
||||
<div class="flex items-center justify-between">
|
||||
<div class="flex items-center space-x-3">
|
||||
<i class="fas fa-rotate text-lg"></i>
|
||||
<span class="text-sm font-medium" aria-live="polite">
|
||||
<span class="text-sm font-medium" aria-live="polite"
|
||||
id="restart-pending-text">
|
||||
Configuration saved — restart the display to apply the changes
|
||||
</span>
|
||||
</div>
|
||||
@@ -1107,15 +1108,29 @@
|
||||
fetch('/api/v3/system/check-update')
|
||||
.then(function(r) { return r.json(); })
|
||||
.then(function(data) {
|
||||
var banner = document.getElementById('update-banner');
|
||||
var btn = document.getElementById('update-banner-btn');
|
||||
if (data.check_failed) {
|
||||
// A check that could not run is not the same as being up
|
||||
// to date. Hiding the banner here made a checkout git
|
||||
// refuses to touch look permanently current, with no
|
||||
// route to the update button and nothing to act on.
|
||||
document.getElementById('update-banner-text').textContent =
|
||||
data.error || 'Could not check for updates.';
|
||||
if (btn) btn.style.display = 'none';
|
||||
banner.style.display = '';
|
||||
return;
|
||||
}
|
||||
if (btn) btn.style.display = '';
|
||||
if (data.update_available && getDismissedSha() !== data.remote_sha) {
|
||||
var n = data.commits_behind || 0;
|
||||
var msg = 'A new LEDMatrix update is available';
|
||||
if (n > 0) msg += ' (' + n + ' commit' + (n > 1 ? 's' : '') + ')';
|
||||
document.getElementById('update-banner-text').textContent = msg;
|
||||
document.getElementById('update-banner').style.display = '';
|
||||
banner.style.display = '';
|
||||
try { sessionStorage.setItem('update-sha', data.remote_sha); } catch(e) {}
|
||||
} else {
|
||||
document.getElementById('update-banner').style.display = 'none';
|
||||
banner.style.display = 'none';
|
||||
}
|
||||
})
|
||||
.catch(function() {});
|
||||
@@ -1146,6 +1161,13 @@
|
||||
if (data.status === 'success') {
|
||||
document.getElementById('update-banner').style.display = 'none';
|
||||
try { sessionStorage.removeItem('update-sha-dismissed'); } catch(e) {}
|
||||
// The pull replaced files on disk; the running services still
|
||||
// hold the code they loaded at boot. Ask for the restart that
|
||||
// makes the update actually take effect.
|
||||
if (data.restart_required && typeof window.showRestartPending === 'function') {
|
||||
window.showRestartPending(
|
||||
'Update installed \u2014 restart the display to run the new code');
|
||||
}
|
||||
}
|
||||
if (typeof showNotification === 'function') {
|
||||
showNotification(data.message || 'Update complete', data.status || 'success');
|
||||
|
||||
@@ -95,7 +95,7 @@
|
||||
<!-- Location Information -->
|
||||
<div class="grid grid-cols-1 md:grid-cols-3 xl:grid-cols-3 2xl:grid-cols-3 gap-4">
|
||||
<div class="form-group" id="setting-general-city" data-setting-key="location.city">
|
||||
<label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, radar, and other location-based content.\nExample: Kansas City.\nUsed as the default for the location_city setting on plugins that have one; a value saved on the plugin itself overrides it.', 'City') }}</label>
|
||||
<label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, and other location-based content.\nExample: Dallas.', 'City') }}</label>
|
||||
<input type="text"
|
||||
id="city"
|
||||
name="city"
|
||||
@@ -104,7 +104,7 @@
|
||||
</div>
|
||||
|
||||
<div class="form-group" id="setting-general-state" data-setting-key="location.state">
|
||||
<label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Missouri. Improves location-lookup accuracy.\nUsed as the default for the location_state setting on plugins that have one.', 'State') }}</label>
|
||||
<label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Texas. Improves location-lookup accuracy.', 'State') }}</label>
|
||||
<input type="text"
|
||||
id="state"
|
||||
name="state"
|
||||
@@ -113,7 +113,7 @@
|
||||
</div>
|
||||
|
||||
<div class="form-group" id="setting-general-country" data-setting-key="location.country">
|
||||
<label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather, radar, and geolocation.\nUsed as the default for the location_country setting on plugins that have one.', 'Country') }}</label>
|
||||
<label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather and geolocation.', 'Country') }}</label>
|
||||
<input type="text"
|
||||
id="country"
|
||||
name="country"
|
||||
|
||||
Reference in New Issue
Block a user