Skip to content

Keep the model cache out of Home Assistant backups, allow /share as backup target - #36

Merged
NerdyHank merged 8 commits into
mainfrom
smaller-ha-backups
Oct 6, 2026
Merged

NerdyHank merged 8 commits into
mainfrom
smaller-ha-backups

Conversation

@NerdyHank

Copy link
Copy Markdown
Contributor

Addresses two of the three points in #33. Stacked on #35, which introduces the map: block both changes build on — merge #35 first and the base retargets to main on its own.

The report came with measurements, which made this easy to act on:

part of the app backup size
image.tar (built locally, so stored every time) 1,448 MB
data/model-cache/.insightface 601 MB
gallery, history, settings — the irreplaceable part ~11 MB

What this does

backup_exclude for the model cache — roughly 600 MB that re-downloads by itself on the next start. Nothing in it can be lost.

backup_dir as an app option, /share mounted writable. Point the built-in daily gallery backup at /share/faceid and it lands where Home Assistant's own backups already look, at a few MB. The gallery is the one thing that cannot be re-created, and until now it only lived inside the app's data volume.

/media deliberately stays read-only. It is where recordings are read from, never written to; only the backup target needed write access. There is a test pinning that distinction so it cannot drift.

Not addressed

The image itself, ~1.4 GB per backup, because the app is built locally rather than pulled from a registry. Fixing it means publishing prebuilt images for amd64 and aarch64 and setting image: — a build pipeline and a registry, not a config line. Left in #33 as the remaining item, and noted in DOCS.md so nobody expects the backup to be 11 MB yet.

76 tests green.

🤖 Generated with Claude Code

…ackup target

Reported in #33 with measurements: a backup of the app is about 1.1 GB
compressed, while the data that cannot be re-created is about 11 MB.

- backup_exclude drops the model cache, roughly 600 MB that re-downloads by
  itself on the next start.
- backup_dir is now an app option, and /share is mounted writable so the
  built-in gallery backup can be written where Home Assistant's own backups
  already look. /media stays read-only: it is a source, never a target.

Not addressed here: the app image itself, about 1.4 GB in every backup because
the app is built locally rather than pulled. That needs prebuilt images and a
registry, which is a separate decision.

76 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@the-codemole

the-codemole Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

🧪 Automated PR Checks

Profile: python-app · auto-detected · ⚙ Configurable

✅ python-syntax — 6 Python file(s) compile cleanly
✅ ruff — 6 Python file(s) — ruff clean (E9/F)
⚪ json-valid — ⏭️ No JSON files in the diff
✅ diff-size — Diff +596/-21 in 13 files
✅ secret-scan — No plaintext secrets in the added lines
✅ conflict-markers — No merge conflict markers
✅ sensitive-files — No sensitive file types in the diff

hermes-work · branch smaller-ha-backups · base main · What do these checks do?

Comment thread faceid-addon/config.yaml
A backup_dir under the add-on's read-only /media mount was accepted
everywhere and then failed once a night inside the backup thread, where
only the log saw it — the kind of failure you discover when you need the
backup. The path is now write-probed in three places: at add-on start-up,
when it is saved in Settings (HTTP 400 with the reason), and on a manual
backup (400 instead of 500). The probe writes a real temporary file and
removes it again, because /media is readable and only the write attempt
reveals the mount is read-only.

The Settings tab and the 'Back up now' button now show the reason instead
of swallowing it: saving an unusable path used to leave the form silent.
@NerdyHank

Copy link
Copy Markdown
Contributor Author

Half right, and the half that is right was worth the fix.

The pruning claim does not hold. prune_backups has always globbed only its own
archives:

files = sorted(backup_dir.glob("faceid-backup-*.tar.gz"), reverse=True)

A shared folder keeps everything else. There is now a test that says so — it drops
family-photos.tar.gz, notes.txt and movie.mkv next to three backups, prunes to
keep=1, and asserts the three unrelated files are still there.

The missing validation was real, and worse than "no validation": a backup_dir under
the read-only /media mount was accepted by the Settings form, by the add-on at start-up,
and by the auto-backup thread, which caught the OSError, wrote one line to the log and
carried on. That is a failure you find out about when you need the backup. Fixed in three
places, all of them write-probing with a real temporary file — /media is readable, so
os.access and -d both say yes and only the write attempt reveals the mount:

  • check_backup_dir() in backup_util.py: absolute path, creatable, directory, writable;
    returns the reason or None, and leaves nothing behind.
  • POST /api/settings refuses an unusable path with HTTP 400 and the reason, so nothing is
    stored. POST /api/backup/now turns the OSError into a 400 instead of a 500.
  • run.sh probes it at start-up and exits with the way out named:
    /media is mounted read-only; put the backup under /share (e.g. /share/faceid).

The UI swallowed it too — saveBackupCfg() had no catch, so an invalid path produced no
toast at all. It now shows the reason, as does "Back up now".

Measured against the running router on a spare port (no MQTT, no Frigate, separate data
directory):

read-only dir : 400 backup_dir unusable: cannot write to …/readonly: Permission denied
relative path : 400 backup_dir unusable: backups/x is not an absolute path
good dir      : 200 {"applied":{"backup_dir":"…/good"}}
backup now    : 200 …/faceid-backup-20261006-124724.tar.gz
now on ro dir : 400 cannot write to …/readonly: Permission denied
probe left    : []

and the start-up block as an unprivileged user:

FATAL: backup_dir '…/ro/faceid' cannot be created.
FATAL: Only /media (read-only) and /share (writable) are mounted — use /share/faceid.

88 tests green.

Comment thread app/backup_util.py
Comment thread app/backup_util.py Outdated
Three follow-ups on the write probe:

- A typo like /shre/faceid passed the probe, because the container's overlay
  filesystem is writable — and an archive there is gone after the next add-on
  update, which is the silent loss this check exists to prevent. The add-on
  now exports FACEID_PERSISTENT_ROOTS=/data:/share and run.sh refuses anything
  outside it up front; standalone installs have no such boundary and stay
  unrestricted.
- Validation no longer leaves directories behind: the path is checked before
  anything is created, and a failed probe removes the (empty) directories the
  check itself made.
- The probe file is written inside a with block. Without it a raising write()
  skipped close(), leaking the descriptor and leaving .faceid-write-test-* in
  the target directory on every save.
Comment thread app/backup_util.py
Path.parents is lexical, so /share/../config/faceid listed /share among its
parents, passed the containment check and then had mkdir and the probe write
land in /config. A symlink inside an allowed mount escaped the same way. The
path is now resolved first, and the roots with it so a symlinked /share does
not reject its own subtree; the error message names both the given and the
resolved path, because a rejection that only shows the typed path is hard to
act on.
@NerdyHank
NerdyHank deleted the branch main October 6, 2026 11:24
@NerdyHank NerdyHank closed this Oct 6, 2026
the-codemole[bot]
the-codemole Bot previously approved these changes Oct 6, 2026

@the-codemole the-codemole Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — review complete.

  • Checks: 6 passed, 1 skipped, 0 failed
  • 4 finding(s) addressed, no open threads

@NerdyHank NerdyHank reopened this Oct 6, 2026
@NerdyHank
NerdyHank changed the base branch from folder-mode-in-ha-app to main October 6, 2026 11:25
Two defects in the 0.25.0 entry: it claimed /media and /share are both
mounted read-only, which the backup target contradicts — /share is
writable, and only for that purpose. And the write-probe plus the
persistent-mount requirement were not mentioned at all, although they
change what the add-on accepts and will refuse to start on.
Comment thread app/webui.py
@the-codemole
the-codemole Bot dismissed their stale review October 6, 2026 11:26

No longer clean — approval withdrawn.

write_backup_file streamed straight into faceid-backup-<ts>.tar.gz, so a
write that broke off mid-way (ENOSPC, EIO) left a truncated archive behind
— and being the newest, it made prune_backups evict a valid older backup.
The archive is now written under a dot-prefixed .part name the rotation does
not glob, and moved into place with os.replace() only once complete; a
failure unlinks it.

prune_backups also sat outside the try in POST /api/backup/now, so an
OSError from the rotation still produced the HTTP 500 the handler was
supposed to replace.

@the-codemole the-codemole Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — review complete.

  • Checks: 6 passed, 1 skipped, 0 failed
  • 5 finding(s) addressed, no open threads

Two defects. The entry claimed /media and /share are both mounted
read-only, which the writable backup target contradicts. And the write
probe, the persistent-mount requirement, the atomic archive write and the
folder-camera discovery fix were not mentioned at all, although they
change what the add-on accepts and what it refuses to start on.

Written in CHANGELOG.md this time: faceid-addon/CHANGELOG.md is a copy that
scripts/sync-addon.sh overwrites, so two earlier attempts there were lost
on the next sync.
@NerdyHank
NerdyHank merged commit ebea659 into main Oct 6, 2026
1 check passed
@NerdyHank
NerdyHank deleted the smaller-ha-backups branch October 6, 2026 11:30

@the-codemole the-codemole Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — review complete.

  • Checks: 6 passed, 1 skipped, 0 failed
  • 5 finding(s) addressed, no open threads

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants