Files
joulenap/backend/app/api/_config_edit.py
T
Catubba 0ad459e14c fix(backend): resolve the nine findings of the backend-block review
The three silent ones first. A 0.9 config that fails to convert used to boot an
empty config that looks exactly like a fresh install: the reason now reaches the
UI (GET /api/status.config_error), the activity log and an ERROR line, and the
.bak parachute is written on the failure branch too, so a later save from the
Advanced tab cannot destroy the original.

"PBS left powered on" was wrong in both directions -- every successful run
against an always-on PBS warned, and a sync route that left its target awake did
not. lease.release() returned one False for four situations, only two of which
cost power; it now returns a ReleaseOutcome that names the reason, which becomes
both the POWEROFF step's detail and RunContext.left_on. The interrupted-run path
keeps a step-derived rule, now paired per device and filtered by managed_power.

422 bodies echoed the whole config, secrets included: a config-level validator
raises at loc=(), so pydantic attached every token, the secret key, the password
hash, the SMTP and bot tokens as the error's input. One helper with
include_input=False now serves all three config-shaped 422 sites.

Also: a redaction placeholder with nothing to resolve against is rejected instead
of silently clearing the credential (a renamed device id, or a create from a
copied body); the ad-hoc "Run verify" asks for outdated_after=0, since None meant
"only never-verified" and skipped exactly the snapshots the button exists for;
the manual power-off holds the single-run lock so it cannot cut a vzdump that
started in the check-then-act gap; _current_run_id is cleared when a run ends, so
a stop landing between two runs cannot hit the wrong one; and the pre-migration
.bak is chmod 0600 like every other secret-bearing file.

Tests: 617 passed, 2 skipped. Every finding was reproduced against the real code
before the fix, and each new test confirmed failing on the pre-fix code.
2026-08-03 00:56:40 +02:00

89 lines
3.7 KiB
Python

"""One way to change a piece of config: validate the whole document, persist, re-arm.
The route and device endpoints all edit one list inside ``config.yaml``. Doing that by
mutating the live model would skip :meth:`Config._check_references` — the cross-checks that
catch a route pointing at a device that doesn't exist, or a duplicate id. So every edit is
"dump, replace one list, re-validate the whole thing", which is cheap (the config is a few
kilobytes) and makes an invalid combination impossible to persist rather than merely
unlikely.
"""
from __future__ import annotations
from typing import Any
from fastapi import HTTPException, status
from fastapi.encoders import jsonable_encoder
from pydantic import ValidationError
from ..config import Config
from ..core.config_store import ConfigStore
from ..core.scheduler import Scheduler, validate_cron
def validation_error(exc: ValidationError) -> HTTPException:
"""A 422 carrying pydantic's error list — **without** the values that failed.
``include_input=False`` is the whole point: a ``model_validator(mode="after")`` on
``Config`` raises at ``loc=()``, so pydantic attaches the entire validated config as the
error's ``input``. Serialised, that ships every API token, ``app.secret_key``, the
password hash, the SMTP password and the bot token to the browser's network tab and any
proxy log in between. The config-level cross-checks (a route pointing at a device that
doesn't exist, an External route onto an unmanaged PBS) are ordinary user mistakes, so
this is a body users really do see. ``include_url`` only drops a docs link.
Every config-shaped 422 goes through here so a new endpoint inherits the safe default.
"""
return HTTPException(
status_code=422, # the literal avoids the deprecated HTTP_422_UNPROCESSABLE_ENTITY
detail=jsonable_encoder(exc.errors(include_input=False, include_url=False)),
)
def save_section(
store: ConfigStore, scheduler: Scheduler, section: str, value: list[dict[str, Any]]
) -> Config:
"""Replace ``config.<section>`` with ``value``, validate, persist and re-arm.
Raises 422 with pydantic's own error list when the result doesn't validate, and 500 when
the file can't be written (a read-only mount).
"""
raw = store.config.model_dump(mode="python")
raw[section] = value
try:
new_config = Config.model_validate(raw)
except ValidationError as exc:
# 422 mirrors FastAPI's own body-validation responses.
raise validation_error(exc) from exc
check_route_crons(new_config, store.config)
try:
store.replace(new_config)
except OSError as exc:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=str(exc)
) from exc
scheduler.rearm(new_config)
return new_config
def check_route_crons(new_config: Config, old_config: Config) -> None:
"""Reject a newly-set unparseable ``schedule.cron`` before it reaches disk (BE-B1).
An invalid string wouldn't crash arming — ``_arm_route`` guards it — but the route would
silently never fire, which is the failure mode hardest to notice. Only *changed* values
are checked, so a legacy string already on disk doesn't lock the user out of saving an
unrelated edit.
"""
old = {r.id: r.schedule.cron for r in old_config.routes}
for route in new_config.routes:
cron = route.schedule.cron
if not cron or cron == old.get(route.id):
continue
try:
validate_cron(cron)
except (ValueError, TypeError) as exc:
raise HTTPException(
status_code=422,
detail=f"route '{route.id}': invalid schedule.cron {cron!r}: {exc}",
) from exc