Follow-up to review feedback on #7003 and #7004: when removing the
hosts file in CoreDNS.reset() or cleaning up a backup tarfile in
BackupManager.import_backup(), pass any OSError to
sys_resolution.check_oserror() so known filesystem issues (e.g.
EBADMSG) mark the system unhealthy early. For the backup paths the
check only applies to local locations (default and cloud backup),
matching the existing handling in remove() and _copy_to_location().
This also covers the second unlink in import_backup() (cleanup after
a failed load) and stops OSError from bubbling out of both cleanup
paths unhandled to the job: the error is now logged, and the method
still returns None or raises the original BackupInvalidError.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix typos in comments, docstrings and log messages
Correct 39 spelling mistakes across comments, docstrings and log/error
message strings throughout the package (e.g. "conection" -> "connection",
"Incomming" -> "Incoming", "Rasie" -> "Raise"). All changes are confined
to human-readable text; no identifiers, attributes or D-Bus contracts are
touched, so there is no behavior change. Found with codespell.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix typos in tests and CI workflow
Correct spelling mistakes in test comments, docstrings and data, plus one
in the builder workflow, so the whole tree is clean for the codespell hook
added next. The assertion in test_network_manager.py is updated to match
the corrected "Unknown error while processing" log message in the source.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add codespell pre-commit hook
Wire up codespell so spelling mistakes in comments, docstrings and strings
are caught automatically. The vendored frontend panel is excluded, and
"hass" and "astroid" are added to the ignore list as known false positives
(the Home Assistant abbreviation and the pylint dependency package).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address review feedback
Improve grammar in several of the touched comments and docstrings: use the
plural "ignore conditions" for the list-returning property, add the missing
auxiliary verb and fix agreement in the timezone-filter comment, fix
"backups ... use" agreement, and reword "underlay" to "underlying" in the
arch module docstring.
Also drop the "*.json" skip from the codespell hook. It was carried over
from another project but is unnecessary here (all tracked JSON is clean),
and skipping it would needlessly leave translation and data JSON unchecked.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Reword onboarding comment
"overflight" was a literal calque of the German "überflogen"; use the
idiomatic "skimmed through" instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Derive App state from container state
The App.state setter mixed two responsibilities: it both mutated a
private `_state` field and dispatched side effects (WebSocket events,
issue dismissal, startup_event signaling). On top of that, an installed
but never-started app stayed in AppState.UNKNOWN forever, because the
attach() image-only fallback never fires a container state-change event
and the AppState therefore kept its constructor default. Conceptually,
ContainerState.UNKNOWN ("container does not exist") and AppState.UNKNOWN
("nothing observed yet") happened to share a name but meant different
things, which made the distinction easy to lose.
Make App.state a pure derived property. The source of truth is the last
observed ContainerState (cached on the App), plus a sticky operation-
error flag for start/stop failures that the docker event stream cannot
reflect. When no container has been observed yet, the derivation falls
back to install signals: an attached instance (image present) is
STOPPED, otherwise UNKNOWN. As a side effect, an installed-but-never-
started app now correctly reports STOPPED instead of UNKNOWN.
container_state_changed updates the cached container state and routes
all side effects through a single _emit_state_change helper that diffs
old vs new derived state. The two start/stop failure paths route
through _set_operation_error. Uninstall resets the cached signals so
the derivation naturally returns UNKNOWN.
Tests use a new tests/common.force_app_state helper that pokes the
underlying signals directly; the production class no longer carries
test-only setters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix App state drive to AppState.UNKNOWN
* Unify state mutation through _update_state
Previously, state-driving signal changes were spread across two helpers
(_set_operation_error, _emit_state_change(old_state)) and required each
caller to capture self.state before mutating a private field — leaking
implementation details to call sites and raising the "why am I emitting
the old state?" question pointed out in code review.
Replace both helpers with a single _update_state(*, container_state=,
operation_error=) entry point. Callers describe what changed via
keyword arguments (None leaves a signal untouched); the helper captures
the previous state, applies the updates, recomputes the derived state
and emits side effects if anything changed.
Diff against a tracked _last_state instead of a freshly derived
"current" state, so that an out-of-band mutation between updates does
not silently shift the comparison baseline. The concrete case is
App.uninstall: instance.remove() clears the docker meta mid-flow, which
would otherwise reshape the derivation (RUNNING with no healthcheck
becomes STARTED instead of STARTUP) and suppress the STARTUP transition
that resolves the start-wait task. As a side effect, the initial
UNKNOWN -> STOPPED transition on attach is also now reliably emitted.
Switch the uninstall path to ContainerState.UNKNOWN ("we know there is
no container") rather than the constructor sentinel None.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Cache app state instead of deriving on every read
Building on the previous commit, make App.state a plain read of a
cached _state field rather than re-deriving on every property access.
The derivation moves to _derive_state(), and _update_state() is the
sole place that recomputes and assigns _state, so the value consumers
read always matches what was last emitted to listeners.
This removes the _last_state bookkeeping introduced previously: with a
single cached value there is no longer a separate "derived now" vs
"last emitted" distinction to reconcile, and out-of-band mutations
(e.g. instance.remove() clearing _meta during uninstall) can no longer
silently shift what state returns between updates.
Call _update_state() at the end of load() so the cached state settles
once attach() has run. Image-only attaches do not fire a docker event,
so without this an installed app would stay in the constructor-default
UNKNOWN until first start; this also makes the initial transition on
attach observable to listeners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pass operation error to _derive_state instead of storing it
The two state-driving signals were not symmetric. _container_state is
genuinely persisted state ("the last thing docker told us") that
re-derivation legitimately reads across calls. _operation_error, on the
other hand, is a momentary "force ERROR for this transition" signal; the
persistence of an error condition already lives in the cached _state.
Storing it as an instance attribute implied a sticky cross-call behavior
that no call path actually exercised: every caller either set it
explicitly right before deriving (start/stop failures, container events)
or ran argless only at load time, where no failure has occurred.
Drop the _operation_error field and pass operation_error as a parameter
to _derive_state(), defaulting to False in _update_state(). A container
observation now supersedes a prior error implicitly via the default,
which lets the container-event and uninstall call sites drop their
explicit operation_error=False.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle load state synchronously from current_state
The argless _update_state() settle at the end of load() raced attach()'s
container-state event. attach() fires DOCKER_CONTAINER_STATE_CHANGE via
the bus, which schedules the container_state_changed listener as a task
rather than running it inline. In the deprecated-arch early-return path
there is no await between attach() and the settle, so the listener had
not run yet: _container_state was still None and the settle derived
STOPPED (instance attached) — emitting a transient UNKNOWN->STOPPED even
for a running container before the listener corrected it. The main path
only avoided this incidentally, by having awaits (check_image,
save_persist) in between for the listener to run.
Derive the load-time state synchronously from instance.current_state()
instead of relying on the asynchronously delivered event. current_state()
returns the real container state, or UNKNOWN when only an image is
present (which derives to STOPPED), so both paths settle correctly
without racing the event.
Add a regression test that loading a running container settles to
STARTED, and mock current_state() in the state-listener test which
relies on a clean UNKNOWN baseline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* apps: avoid blocking is_file() check in restore
Restoring an app off a backup called `Path.is_file()` directly on the
event loop to check whether the bundle contained an `image.tar`, which
triggered a blockbuster BlockingError on `os.stat` in environments where
blocking-call detection is enabled. Wrap the check in
`self.sys_run_in_executor(...)` so the stat happens off the main thread,
matching the pattern already used in `api/backups.py`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* tests: cover restore path where addon image is missing
The blocking ``is_file()`` call that the previous commit replaced lived
in the ``not instance.exists()`` branch of ``App.restore()`` -- reached
only when the local Docker image for the addon is absent. Existing
app-restore tests start from ``install_app_ssh`` / ``install_app_example``
and rely on the mocked ``docker images.inspect`` which always reports
the image as present, so that branch never executed and blockbuster
never had a chance to flag the blocking stat.
Add a focused test that uninstalls the app after backup, patches
``DockerApp.exists`` to ``False``, and stubs ``install`` / ``import_image``
so the restore drives through the ``image.tar`` check. Verified that
this test raises ``BlockingError: Blocking call to os.stat`` when run
against the unpatched code.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Turn on the PTH rule set in ruff and convert the remaining call sites
that triggered violations:
- `supervisor/utils/yaml.py` and `tests/homeassistant/test_module.py`
use `Path.open()` instead of the builtin `open()`.
- `tests/backups/test_backup.py` and
`tests/resolution/fixup/test_store_execute_reset.py` use
`Path.iterdir()` instead of `os.listdir()`.
- `tests/mounts/test_mount.py` uses `Path.unlink()` instead of
`os.remove()`.
- `tests/store/test_translation_load.py` uses `Path.mkdir(parents=True)`
instead of `os.makedirs()`.
- `tests/hardware/test_disk.py` captures the unpatched `Path.is_dir`,
`Path.is_symlink` and `Path.stat` before the `patch.object(Path, ...)`
block so the mocks can delegate to the real implementations without
reaching for `os.path.isdir`/`os.path.islink`/`os.stat`.
* tests: enable flake8-pytest-style (PT) ruff rules
Enable the `PT` ruff rule set and fix the resulting violations across the
test suite:
- PT006: pass parametrize argument names as tuples instead of a single
comma-separated string.
- PT022: switch fixtures that have no teardown from `yield` to `return`
so the lack of cleanup is obvious at a glance.
- PT011: add `match=` to broad `pytest.raises(ValueError)` blocks so the
expected error is anchored to a specific message.
- PT012: hoist setup (patches, branching) out of `pytest.raises()`
blocks so only the call that is expected to raise remains inside.
- PT013: replace `from pytest import X` with `import pytest` and access
attributes via the module.
- PT015: replace `try/except` + `assert False` patterns with
`pytest.raises(...)`.
- PT017: replace `assert` on exceptions inside `except` blocks with
`pytest.raises(...) as exc_info` and assert on `exc_info.value`.
No behavioral changes to the tests; the full suite still passes.
* tests: address review feedback on PT ruff rule enablement
- Fix fixture return-type annotations after switching `yield` to `return`
in tests/conftest.py: drop the `Generator[...]`/`AsyncGenerator[...]`
wrapper for `dns_manager_service`, `supervisor_internet`, `websession`,
and `mock_update_data` so the annotation matches what the fixture
actually returns.
- Correct the return-type annotation of `fixture_ip6config_service` from
`IP4ConfigService` to `IP6ConfigService`.
- Fix recurring "excepiton" typo in tests/utils/test_exception_helper.py.
* tests: verify backup cleanup on permission error
After `test_new_backup_permission_error` raises `BackupPermissionError`,
assert that no tarfile was left behind and `tmp_path` is empty. The
previous version only checked that the exception was raised, which
missed any regression where a partial tarfile would survive the failed
create.
* tests: rename DNS_GOOD_V6 to DNS_V6_UNSUPPORTED
The constant was named "good" but its tests assert that the URLs are
rejected by the DNS validator. The IPv6 URLs are well-formed but
currently rejected because IPv6 doesn't work with the Docker network
(see `dns_url` in supervisor/validate.py). Rename the constant and the
related test to make the intent obvious.
* mounts: use softerr for NFS instead of soft
Switch the NFS mount option from `soft` to `softerr`. For HAOS-style
supervisor mounts (media, share, backup — not the root filesystem) the
error semantics matter:
* `softerr` returns `ETIMEDOUT` on timeout instead of `EIO` (`soft`).
`EIO` is indistinguishable from "the disk is dying"; tools like
SQLite, restic, rsync, ffmpeg tend to treat it as a hard storage
failure (mark database corrupt, abort backup with a hard error,
etc.). `ETIMEDOUT` is unambiguously "the network/server is gone,
transient" and is more commonly handled as retry-later. Supervisor
can also surface a clear "server unreachable" notification rather
than a generic I/O error.
* `softerr` was added in kernel 5.10 precisely to give the fail-fast
behavior of `soft` with a distinct errno so well-behaved apps can
do the right thing.
* For writes-must-not-be-lost use cases (databases, paid storage,
evidence-grade logging) one would want `hard,intr` and a different
recovery story. HAOS NFS mounts are not that — they're add-on
storage where "the share went offline, try again later" is the
correct user-visible behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* mounts: make is_mounted honest about server reachability
systemd's "active/mounted" state is derived from /proc/self/mountinfo
and doesn't reflect whether the backing server actually answers. For
CIFS in particular, smb3_reconfigure never contacts the server, so a
reload of a dead share returns active/mounted with no recovery
attempted. PR #4882 added a Path.is_mount() cross-check to catch this,
but Path.is_mount() relies on os.stat() of the mountpoint root — and
both NFS (softreval) and CIFS (cached root inode attrs) can serve that
stat from local state without going to the wire, so it lies in exactly
the dead-share scenario it was meant to detect.
The visible failure: an API reload of a backup mount whose server had
gone away "succeeded", supervisor then kicked off sys_backups.reload()
against the dead share, and the executor fanned out hundreds of
"Task exception was never retrieved" OSError(112) tracebacks as each
backup tarball's stat() parked in the kernel and failed.
Replace the local-state checks with a statvfs() probe in
NetworkMount.is_mounted(). os.statvfs() returns per-filesystem data
(free blocks, total blocks) that has no client-side cache in either
kernel — neither cifs_statfs() nor nfs_statfs() has an early-return on
cache freshness; both build and send a real FSSTAT / QUERY_FS_INFO
request. So the kernel either reaches the server or gives up with
ETIMEDOUT / EHOSTDOWN / ECONNABORTED. is_mounted() now reflects actual
reachability, finally fulfilling PR #4882's stated intent.
With is_mounted() honest, the reload/restart machinery falls out
naturally: Mount.reload()'s fast path is just `if await self.is_mounted()`,
the post-reload check is the same call, and the "reload succeeded
systemd-wise but probe failed" branch collapses into the existing
"not mounted after reload, try restart" branch. mount(), _restart()
and update() already call is_mounted(); they now get the probe for
free.
To make the probe meaningful, the network mount option strings get
explicit kernel-side timeouts:
* NFS switches from `soft` to `softerr` so timeouts surface as
ETIMEDOUT rather than EIO. EIO is indistinguishable from
"disk dying" and gets misinterpreted by SQLite/restic/rsync as a
hard storage failure; ETIMEDOUT is unambiguously transient.
* CIFS gains `soft,echo_interval=10,retrans=0`, giving a ~30s
per-operation detection budget (3 x echo_interval since last server
response) that matches the NFS budget from `timeo=100,retrans=2`.
Both protocols now fail bounded operations in roughly the same time.
The probe is intentionally not wrapped in an asyncio timeout: the
kernel-side bound is authoritative, and an asyncio timeout would only
orphan the executor thread without unblocking the syscall. The probe
emits debug logs with timing so the ~30s syscall wait on a dead share
is visible in LOGLEVEL=debug traces instead of appearing as a hang.
Tests:
* mock_is_mount fixture extended to also patch os.statvfs so existing
tests that rely on a healthy mount don't need to know about the
probe.
* New manager test split into _healthy_skips_systemd (probe succeeds,
fast path, no systemd call) and _probe_failure_triggers_systemd_reload
(probe fails, escalation runs). API reload test covers both paths.
* Existing tests that simulated "mount is down" via
mock_is_mount.return_value=False updated to simulate probe failure
via OSError(EHOSTDOWN), since is_mount is no longer the signal.
* mounts: drive systemd job waits via JobRemoved instead of state polling
`_update_state_await` had a race that has been present since network
mounts were introduced (#4269) and survived every subsequent rewrite
(#4733, c75f36305): the wait function reads the unit's ActiveState
*after* the systemd dispatch returns, then matches against a target
set that almost always includes the pre-call state (ACTIVE). If
systemd has not yet started dispatching the queued job — and for
fast operations like CIFS reload (smb3_reconfigure completes in
milliseconds with no server contact) it routinely has not — the wait
matches immediately on the pre-call state and returns "done" before
the operation has actually begun. The race was invisible until the
probe commit started doing honest network work after the wait
returned, at which point we spent a full ~30s NFS probe wait on a
mount that systemd was still in the middle of restarting.
Switch the job-dispatching paths (mount/unmount/reload/restart) to
systemd's `JobRemoved` Manager signal. The pattern:
async with sys_dbus.systemd.job_removed() as jobs:
job_path = await sys_dbus.systemd.restart_unit(...)
await jobs.wait_for_job(job_path)
is structurally race-free: subscription is set up before dispatch,
the job_path is allocated synchronously inside the dispatch call,
and any JobRemoved for that path after subscription is queued. Even
for operations that complete faster than we can process the dispatch
return value (the CIFS reload case), the signal cannot be missed.
`_update_state_await` is kept for `load()` — that path observes
existing state without having dispatched a job, so the
PropertiesChanged-driven wait is the right primitive there.
* mounts: verify the path is a mount point before trusting statvfs
The userspace probe trusted statvfs() alone to declare a network mount
healthy. statvfs is uncacheable client-side for both NFS and CIFS, so
on a real mount it forces a wire RPC — but if the mount is gone from
the kernel mount table (e.g., after a restart cycle whose umount
succeeded but whose mount step failed, leaving the unit ACTIVE in
systemd's view), statvfs() operates on a plain directory on the
underlying root filesystem and happily returns the root fs's stats.
The mount is reported healthy when in fact it doesn't exist.
Add a pre-check using `Path.is_mount` — a parent-vs-path `st_dev`
comparison via stat() — to detect "this path is not actually a mount
point" before issuing statvfs. For the ghost-mount case (path on the
root fs) both stats are local and return immediately. For a real
mount the path-stat may cross into the filesystem driver, but the
result is correct in either case and the statvfs that follows
catches server-unreachable mounts that is_mount can't.
The full is_mounted contract is now: ACTIVE per systemd, present as
a mount point in our namespace, and server-reachable per statvfs.
* mounts: skip wasted probes when systemd already reported job failed
JobRemoved tells us whether the systemd job completed successfully —
we just weren't using it. The previous reload/restart paths ran their
post-job probe unconditionally, including the case where systemd had
already told us the operation failed. On a dead NFS share the kernel
is in transport-reconnect churn for tens of seconds after a killed
mount helper exits, so that probe takes 90+ seconds — and confirms
exactly what systemd already said. Plain wasted time.
* `Mount.reload()` now uses the systemd job result. On "done" we
still probe (CIFS reload is local-only — smb3_reconfigure returns
"done" even against a dead server, so the probe is the only check
that catches the lying-CIFS case). On anything else (failed,
timeout, canceled, dependency, skipped, or our own timeout
returning None) we escalate directly to `_restart()` without
probing.
* `Mount._restart()` skips the probe entirely on non-"done" results.
The mount is definitively not active in that case and the probe
would just spend another 30-90s on a dead share.
Also bump the JobRemoved wait timeout from UPDATE_STATE_TIMEOUT (40s,
sized for a single-helper invocation) to a new SYSTEMD_JOB_TIMEOUT
(90s). RestartUnit runs as stop + start, each bounded by the unit's
TimeoutSec (35s from #6834), so the worst case is ~70s plus systemd
queue dispatch. The previous 40s budget caused supervisor to time
out exactly one second before JobRemoved fired in the observed
dead-NFS restart cycle. UPDATE_STATE_TIMEOUT is kept at 40s for the
PropertiesChanged-driven wait in `load()`, where the layered timeout
invariant from #6827 still applies.
* dbus: filter signals at the wrapper level, drop JobRemovedSignal class
Mike's review on #6838: instead of carrying a bespoke JobRemovedSignal
context-manager class, push the filtering concept into the generic
DBusSignalWrapper. Any caller that subscribes to a broadcast signal
but only cares about specific payloads can pass a predicate.
* DBus.signal() gains an optional `message_filter: Callable[..., bool]`.
* DBusSignalWrapper.wait_for_signal() loops past messages where the
filter returns False, returning the next match.
* JobRemovedSignal goes away. supervisor/dbus/systemd.py exposes a
small factory `job_removed_filter(get_job_path)` that returns the
matching predicate; the getter indirection lets callers subscribe
before the dispatch returns the job path (which is required to be
race-free — see the previous commit).
Mount._run_systemd_job() switches to:
job_path: str | None = None
async with systemd.connected_dbus.signal(
DBUS_SIGNAL_SYSTEMD_JOB_REMOVED,
job_removed_filter(lambda: job_path),
) as signal:
job_path = await dispatch
_id, _path, _unit, result = await signal.wait_for_signal()
The behavior is identical to the previous JobRemovedSignal-based
implementation. The signal queue still buffers messages received
after AddMatch is installed, so subscribing before dispatch keeps
the race-free guarantee.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Rename addon/addons to app/apps in filenames and imports
Continues the addon→app terminology migration (#6786).
Renames all source files, test files, fixture files, and
directories that contained 'addon'/'addons' in their names,
and updates all imports accordingly.
Resolution check files in supervisor/resolution/checks/ that were
renamed override the slug property to preserve the existing API
contract (slugs are exposed via the resolution info API and used
to run checks by name).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Rename add-on.json fixture
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Replace fixed-duration sleeps after bus events with gather
Several tests use ``await asyncio.sleep(...)`` to "wait for the
listener to run" after firing a bus event. The fixed duration is
real wall-clock time and the wait can be indeterministic — if the
handler chain happens to need slightly more time on a busy CI
runner, the assertion races the handler.
``Bus.fire_event`` returns the listener tasks since #6252; capture
and ``await asyncio.gather(*tasks)`` instead of sleeping. Touches
test_bus.py (the bus tests were poking scheduling instead of
verifying their assertions), test_home_assistant_watchdog.py,
test_plugin_base.py, addons/test_manager.py, docker/test_addon.py,
and test_store_execute_reload.py.
Other cleanups in the same spirit:
- ``_fire_test_event`` in addons/test_addon.py becomes ``async def``
and gathers the listener tasks itself, so its 17 call sites
collapse to a single ``await _fire_test_event(...)``.
- The two test_store_execute_reload.py sites that used the private
``_update_connectivity()`` helper are reworked to set the cached
connectivity flag directly and fire the event themselves so they
can gather the listener tasks the same way.
- The two ``sleep(1)`` post-pull drains in docker/test_interface.py
collapse to ``sleep(0)`` (handler tasks are already gathered
inside pull_image), saving ~2s.
- The ``sleep(0.01)`` waits inside ``container_events()`` task
bodies (api/test_addons.py, api/test_store.py,
backups/test_manager.py) are just one-yield-to-the-parent and
become ``sleep(0)``.
Switching to ``gather`` exposes a few latent test mocks that were
silently swallowing TypeErrors as background-task failures before:
- ``CGroup.add_devices_allowed`` is ``async def`` but was patched
as a plain MagicMock in docker/test_addon.py — now patched via
``new_callable=AsyncMock``.
- The watchdog does ``await (await self.start())`` /
``await (await self.restart())`` because ``App.start`` /
``App.restart`` return ``asyncio.Task``. The mocks in
addons/test_addon.py (test_app_watchdog, test_watchdog_on_stop,
test_watchdog_during_attach) needed
``AsyncMock(return_value=<settled future>)`` to mirror that
shape rather than a plain MagicMock.
* Factor bus.fire_event + gather pattern into a helper
Per review feedback, the ``await asyncio.gather(*coresys.bus.fire_event(...))``
incantation was scattered across many call sites. Add
``tests.common.fire_bus_event`` that takes the coresys, event and data,
fires the event and awaits the spawned listener tasks. Convert all
matching sites to use it, including the ``_fire_test_event`` wrapper
in addons/test_addon.py which now just builds the
``DockerContainerStateEvent`` and delegates.
* Rename addon→app in docstrings and comments
Updates all docstrings and inline comments across supervisor/ and
tests/ to use the new app/apps terminology. No runtime behaviour
is changed by this commit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Rename addon→app in code (variables, args, class names, functions)
Renames all internal Python identifiers from addon/addons to app/apps:
- Variable and argument names
- Function and method names
- Class names (Addon→App, AddonManager→AppManager, DockerAddon→DockerApp,
all exception, check, and fixup classes, etc.)
- String literals used as Python identifiers (pytest fixtures,
parametrize param names, patch.object attribute strings,
URL route match_info keys)
External API contracts are preserved: JSON keys, error codes,
discovery protocol fields, TypedDict/attr.s field names.
Import module paths (supervisor/addons/) are also unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix partial backup/restore API to remap addons key to apps
The external API accepts `addons` as the request body key (since
ATTR_APPS = "addons"), but do_backup_partial and do_restore_partial
now take an `apps` parameter after the rename. The **body expansion
in both endpoints would pass `addons=...` causing a TypeError.
Remap the key before expansion in both backup_partial and
restore_partial:
if ATTR_APPS in body:
body["apps"] = body.pop(ATTR_APPS)
Also adds test_restore_partial_with_addons_key to verify the restore
path correctly receives apps= when addons is passed in the request
body. This path had no existing test coverage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix merge error
* Adjust AppLoggerAdapter to use app_name
Co-authored-by: Stefan Agner <stefan@agner.ch>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Let OSError propagate during backup creation
Securetar's create_tar context manager uses a two-phase header write:
on enter it writes a placeholder tar header (size unknown), and on exit
_finalize_tar_entry seeks back to rewrite the header with the actual
size. If an OSError (e.g. ENOSPC) occurs mid-write, the inner tar entry
is left with truncated data and a placeholder header. Continuing to
write more entries on top of this produces a structurally invalid tar
file that cannot be restored.
Previously, _folder_save wrapped OSError as BackupError, which
store_folders then caught and swallowed — allowing the backup to
continue writing to an already corrupt outer tar. Similarly,
_create_finalize silently swallowed OSError when writing backup.json,
and the finally block in create() could raise a secondary OSError from
_close_outer_tarfile that replaced the original exception.
Securetar already distinguishes read vs write errors: read-side errors
(e.g. permission denied on a source file) are wrapped as AddFileError
(non-fatal, skip the file), while write-side OSError propagates as-is.
With this change, write-side OSError is wrapped as BackupFatalError
(a BackupError subclass) instead of plain BackupError. This ensures:
- store_folders/store_addons do not swallow it (they only catch
BackupError, and re-raise BackupFatalError explicitly).
- The job decorator handles it as a HassioError (no extra Sentry
event). Letting OSError bubble up raw would cause the job decorator
to treat it as an unhandled exception, capturing it to Sentry and
wrapping it as JobException — producing more Sentry noise, not less.
- _do_backup catches it via `except BackupError` and deletes the
incomplete backup file. This is the correct behavior since the tar
is structurally corrupt and not restorable.
Changes:
- Add BackupFatalError exception for write-side I/O errors.
- In create(), use except/else instead of finally so that finalization
is skipped when an error already occurred during yield. This prevents
a secondary exception from _close_outer_tarfile replacing the
original error.
- In _create_finalize, raise BackupFatalError on OSError instead of
swallowing it.
- In _folder_save, wrap OSError as BackupFatalError (not BackupError).
- In store_folders, re-raise BackupFatalError instead of swallowing.
- In store_supervisor_config, wrap OSError as BackupFatalError.
Fixes SUPERVISOR-B53
Fixes SUPERVISOR-1FAJ
Fixes SUPERVISOR-BJ4
Fixes SUPERVISOR-18KS
Fixes SUPERVISOR-1HE6
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Rename BackupFatalError to BackupFatalIOError
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Change addons to apps in all user-facing strings
* Fix grammar in errors
* Apply suggestions from code review
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
* Apply suggestions from code review
Co-authored-by: Stefan Agner <stefan@agner.ch>
---------
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Include Docker registry configurations in backups
Docker registry credentials were removed from backup metadata in a prior
change to avoid exposing secrets in unencrypted data. Now that the encrypted
supervisor.tar inner archive exists, add docker.json alongside mounts.json
to securely backup and restore registry configurations.
On restore, registries from the backup are merged with any existing ones.
Old backups without docker.json are handled gracefully.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Increase test coverage by testing more error paths
* Address review feedback for Docker registry backup
Remove unnecessary dict() copy when serializing registries for backup
since the property already returns a dict.
Change DockerConfig.registries to use direct key access instead of
.get() with a default. The schema guarantees the key exists, and
.get() with a default would return a detached temporary dict that
silently discards updates.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Include network storage mount configurations in backups
When creating backups, now stores mount configurations (CIFS/NFS shares)
including server, share, credentials, and other settings. When restoring
from a backup, mount configurations are automatically restored.
This fixes the issue where network storage definitions for backups
were lost after restoring from a backup, requiring users to manually
reconfigure their network storage mounts.
Changes:
- Add ATTR_MOUNTS schema to backup validation
- Add store_mounts() method to save mount configs during backup
- Add restore_mounts() method to restore mount configs during restore
- Add MOUNTS stage to backup/restore job stages
- Update BackupManager to call mount backup/restore methods
- Add tests for mount backup/restore functionality
Fixes home-assistant/core#148663
* Address reviewer feedback for mount backup/restore
Changes based on PR review:
- Store mount configs in encrypted mounts.tar instead of unencrypted
backup metadata (security fix for passwords)
- Separate mount restore into config save + async activation tasks
(mounts activate in background, failures don't block restore)
- Add replace_default_backup_mount parameter to control whether to
overwrite existing default mount setting
- Remove unnecessary broad exception handler for default mount setter
- Simplify schema: ATTR_MOUNTS is now just a boolean flag since
actual data is in the encrypted tar file
- Update tests to reflect new async API and return types
* Fix code review issues in mount backup/restore
- Add bind mount handling for MEDIA and SHARE usage types in
_activate_restored_mount() to mirror MountManager.create_mount()
- Fix double save_data() call by using needs_save flag
- Import MountUsage const for usage type checks
* Add pylint disable comments for protected member access
* Tighten broad exception handlers in mount backup restore
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Address second round of reviewer feedback
- Catch OSError separately and check errno.EBADMSG for drive health
- Validate mounts JSON against SCHEMA_MOUNTS_CONFIG before importing
- Use mount_data[ATTR_NAME] instead of .get("name", "unknown")
- Overwrite existing mounts on restore instead of skipping
- Move restore_mount/activate logic to MountManager (no more
protected-access in Backup)
- Drop unused replace_default_backup_mount parameter
- Fix test_backup_progress: add mounts stage to expected events
- Fix test_store_mounts: avoid create_mount which requires dbus
* Rename mounts.tar to supervisor.tar for generic supervisor config
Rename the inner tar from mounts.tar to supervisor.tar so it can hold
multiple config files (mounts.json now, docker credentials later).
Rename store_mounts/restore_mounts to store_supervisor_config/
restore_supervisor_config and update stage names accordingly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix pylint protected-access and test timeouts in backup tests
- Add pylint disable comment for _mounts protected access in test_backup.py
- Mock restore_supervisor_config in test_full_backup_to_mount and
test_partial_backup_to_mount to avoid D-Bus mount activation during
restore that causes timeouts in the test environment
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Address agners review feedback
- Change create_inner_tar() to create_tar() per #6575
- Remove "r" argument from SecureTarFile (now read-only by default)
- Change warning to info for missing mounts tar (old backups won't have it)
- Narrow exception handler to (MountError, vol.Invalid, KeyError, OSError)
* Update supervisor/backups/backup.py
* Address agners feedback: remove metadata flag, add mount feature check
- Remove ATTR_SUPERVISOR boolean flag from backup metadata; instead
check for physical presence of supervisor.tar (like folder backups)
- Remove has_supervisor_config property
- Always attempt supervisor config restore (tar existence check handles it)
- Add HostFeature.MOUNT check in _activate_restored_mount before
attempting to activate mounts on systems without mount support
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Treat empty string password as None in backup restore
Work around a securetar 2026.2.0 bug where an empty string password
sets encrypted=True but fails to derive a key, leading to an
AttributeError on restore. This also restores consistency with backup
creation which uses a truthiness check to skip encryption for empty
passwords.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Explicitly mention that "" is treated as no password
* Add tests for empty string password handling in backups
Verify that empty string password is treated as no password on both
backup creation (not marked as protected) and restore (normalized to
None in set_password).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Improve comment
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Bump securetar from 2025.12.0 to 2026.2.0
Adapt to the new securetar API:
- Use SecureTarArchive for outer backup tar (replaces SecureTarFile
with gzip=False for the outer container)
- create_inner_tar() renamed to create_tar(), password now inherited
from the archive rather than passed per inner tar
- SecureTarFile no longer accepts a mode parameter (read-only by
default, InnerSecureTarFile for writing)
- Pass create_version=2 to keep protected backups at version 2
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Reformat imports
* Rename _create_cleanup to _create_finalize and update docstring
* Use constant for SecureTar create version
* Add test for SecureTarReadError in validate_backup
securetar >= 2026.2.0 raises SecureTarReadError instead of
tarfile.ReadError for invalid passwords. Catching this exception
and raising BackupInvalidError is required so Core shows the
encryption key dialog to the user.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Handle InvalidPasswordError for v3 backups
* Address typos
* Add securetar v3 encrypted password test fixture
Add a test fixture for a securetar v3 encrypted backup with password.
This will be used in the test suite to verify that the backup
extraction process correctly handles encrypted backups.
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Harden backup tar extraction with Python data filter
Replace filter="fully_trusted" with a custom backup_data_filter that
wraps tarfile.data_filter. This adds protection against symlink attacks
(absolute targets, destination escapes), device node injection, and
path traversal, while resetting uid/gid and sanitizing permissions.
Unlike using data_filter directly, the custom filter skips problematic
entries with a warning instead of aborting the entire extraction. This
ensures existing backups containing absolute symlinks (e.g. in shared
folders) still restore successfully with the dangerous entries omitted.
Also removes the now-redundant secure_path member filtering, as
data_filter is a strict superset of its protections. Fixes a standalone
bug in _folder_restore which had no member filtering at all.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Simplify security tests to test backup_data_filter directly
Test the public backup_data_filter function with plain tarfile
extraction instead of going through Backup internals. Removes
protected-access pylint warnings and unnecessary coresys setup.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Switch to tar filter instead of custom data filter wrapper
Replace backup_data_filter (which wrapped data_filter and skipped
problematic entries) with the built-in tar filter. The tar filter
rejects path traversal and absolute names while preserving uid/gid
and file permissions, which is important for add-ons running as
non-root users.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Apply suggestions from code review
Co-authored-by: Erik Montnemery <erik@montnemery.com>
* Use BackupInvalidError instead of BackupError for tarfile.TarError
Make sure FilterErrors lead to BackupInvalidError instead of BackupError,
as they are not related to the backup process itself but rather to the
integrity of the backup data.
* Improve test coverage and use pytest.raises
* Only make FilterError a BackupInvalidError
* Add test case for FilterError during Home Assistant Core restore
* Add test cases for Add-ons
* Fix pylint warnings
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Erik Montnemery <erik@montnemery.com>
* Migrate info and events to aiodocker
* Migrate container logs to aiodocker
* Fix dns plugin loop test
* Fix mocking for docker info
* Fixes from feedback
* Harden monitor error handling
* Deleted failing tests because they were not useful
* Migrate all docker container interactions to aiodocker
* Remove containers_legacy since its no longer used
* Add back remove color logic
* Revert accidental invert of conditional in setup_network
* Fix typos found by copilot
* Apply suggestions from code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Revert "Apply suggestions from code review"
This reverts commit 0a475433ea.
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Send progress updates during image pull for install/update
* Add extra to tests about job APIs
* Sent out of date progress to sentry and combine done event
* Pulling container image layer
* Harmonize folder and add-on backup error handling
Align add-on and folder backup error handling in that in both cases
errors are recorded on the respective backup Jobs, but not raised to
the caller. This allows the backup to complete successfully even if
some add-ons or folders fail to back up.
Along with this, also record errors in the per-add-on and per-folder
backup jobs, as well as the add-on and folder root job.
And finally, align the exception handling to only catch expected
exceptions for add-ons too.
* Fix pytest
* Improve backup upload location determination
For local backup upload locations, check if the location is on the same
file system an thuse allows to move the backup file after upload. This
allows custom backup mounts. Currently there is no documented,
persistent way to create such mounts in with Home Assistant OS
installations, but since we might add local mounts in the future this
seems a worthwhile addition.
Fixes: #5837
* Fix pytests
* Fix mypy issues in backups module
* Fix mypy issues in dbus module
* Fix mypy issues in api after rebase
* TypedDict to dataclass and other small fixes
* Finish fixing mypy errors in dbus
* local_where must exist
* Fix references to name in tests
* Report stage with error in jobs
* Copy doesn't lose track of the successful copies
* Add stage to errors in api output test
* revert unneessary change to import
* Add tests for a bit more coverage of copy_additional_locations
* Finish out effort of adding and enabling blockbuster
* Skip getting addon file size until securetar fixed
* Fix test for devcontainer and blocking I/O
* Fix docker fixture and load_config to post_init
* Handle permission error on backup create
Make sure we handle (write) permission errors when creating a backup.
* Introduce BackupFileExistError and BackupPermissionError exceptions
* Make error messages a bit more uniform
* Drop use of exclusive mode
SecureTar does not handle exclusive mode nicely. Drop use of it for now.
* Avoid IO in event loop when removing backup
* Refactor backup size calculation
Currently size is lazy loaded when required via properties. This
however is blocking the async event loop.
Backup sizes don't change. Instead of lazy loading the size of a backup
simply determine it on loading/after creation.
* Fix tests for backup size change
* Avoid IO in event loop when loading backups
* Avoid IO in event loop when importing a backup
* Validate Backup always before restoring
Since #5519 we check the encryption password early in restore case.
This has the side effect that we check the file existance early too.
However, in the non-encryption case, the file is not checked early.
This PR changes the behavior to always validate the backup file before
restoring, ensuring both encryption and non-encryption cases are
handled consistently.
In particular, the last case of test_restore_immediate_errors actually
validates that behavior. That test should actually have failed so far.
But it seems that because we validate the backup shortly after freeze
anyways, the exception still got raised early enough.
A simply `await asyncio.sleep(10)` right after the freeze makes the
test case fail. With this change, the test works consistently.
* Address pylint
* Fix backup_manager tests
* Drop warning message
* Drop Docker config from Supervisor backup
The Docker config is part of the main backup metadata. Because we
consolidate encrypted and unencrypted backups today, this leads to
potential bugs when restoring a backup.
* Drop obsolete encrypt/decrypt functions
* Drop unused Backup Job stage
* Handle non-existing file in Backup password check too
Make sure we handle a non-existing backup file also when validating
the password.
* Update supervisor/backups/manager.py
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Add test case and fix password check when multiple locations
* Mock default backup unprotected by default
Instead of setting the protected property which we might not use
everywhere, simply mock the default backup to be unprotected.
* Fix mock of protected backup
* Introduce test for validate_password
Testing showed that validate_password doesn't return anything. Extend
tests to cover this case and fix the actual code.
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Avoid reordering add-on repositories on Backup load
The `ensure_builtin_repositories` function uses a set to deduplicate
items, which sometimes led to a change of order in elements. This is
problematic when deduplicating Backups.
Simply avoid mangling the list of add-on repositories on load. Instead
rely on `update_repositories` which uses the same function to ensure
built-in repositories when loading the store configuration and restoring
a backup file.
* Update tests
* ruff format
* ruff check
* ruff check fixes
* ruff format
* Update tests/store/test_validate.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Simplify test
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Make the API return 404 for non-existing backup files
* Introduce BackupFileNotFoundError exception
* Return 404 on full restore as well
* Fix remaining API tests
* Improve error handling in delete
* Fix pytest
* Fix tests and change error handling to agreed logic
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Use version which is treated CalVer by AwesomeVersion
The current dev version `99.9.9dev` is treated as unkown version type
by AwesomeVersion. This prevents the version from comparing with
actual Supervisor versions, e.g. from an exsiting backup file.
Make the development version a valid CalVer version so development
versions can handle non-development backups.
* Bump to year 9999
* Backup protected status can vary per location
* Fix test_backup_remove_error test
* Update supervisor/backups/backup.py
* Add Docker registry configuration to backup metadata
* Make use of backup location fixture
* Address pylint
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Extend backup API with file name field
Allow to specify a backup file name when creating a backup. This allows
for user friendly backup file names. If none is specified, the current
behavior remains (backup file name is the backup slug).
* Check passed file name using regex
* Use custom filename on download only if backup file name is backup slug
* ruff format
* Remove path from location for download file name
Introduce a validate password method which only peaks into the archive
to validate the password before starting the actual restore process.
This makes sure that a wrong password returns an error even when
restoring the backup in background.
François MartinandGitHub
