Skip to content

enh(config): limit backup/restore to specific files#3338

Merged
MichaIng merged 8 commits into
devfrom
enh/backup
May 22, 2026
Merged

enh(config): limit backup/restore to specific files#3338
MichaIng merged 8 commits into
devfrom
enh/backup

Conversation

@MichaIng
Copy link
Copy Markdown
Member

@MichaIng MichaIng commented May 22, 2026

Backup and restore only those files which are actually generated by motionEye, and reasonable to restore: motion.conf, camera-*.conf, and prefs.json. uploadservices.json contains mostly redundant overridden info (dedicated cleanup needed), and refresh + access tokens for Google and Dropbox OAuth2. Restoring expired or revoked tokens does not make sense. If any is invalid, the OAuth2 authorization loop needs to be done in any case. All other files need to be created on console, manually or via motioneye_init. They do not necessarily need to be writable by the daemon, and should hence be consequently excluded.

In case the config path contains more than 100 files, it was assumed to be "a system-wide config directory", and selective motion.conf + camera-*.conf were added to the backup already, indicating that these were the major files this feature was intended for. This commit aligns things by doing selective backup in any case, adding the reasonable/harmless prefs.json (visual preferences for the GUI), and applying the same file selecting for restoring backups.

For easier OS-agnistic filtering, the native Python tarfile module is used instead of the external tar command.

@MichaIng MichaIng added this to the v0.44.0 milestone May 22, 2026
@MichaIng MichaIng force-pushed the enh/backup branch 2 times, most recently from 496132a to 1d09773 Compare May 22, 2026 16:51
@MichaIng MichaIng requested review from Marijn0, Copilot and zagrim May 22, 2026 16:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refines motionEye’s configuration backup/restore mechanism to only include a small, intentionally-restorable set of files, and switches away from invoking the external tar command in favor of Python’s tarfile module.

Changes:

  • Limit backups to motion.conf, camera-*.conf, and (optionally present) prefs.json.
  • Apply the same allowlist when restoring from a backup archive.
  • Replace external tar subprocess usage with in-process tarfile creation/extraction.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
motioneye/config.py Implements selective backup/restore and migrates backup format handling to Python tarfile.
motioneye/locale/motioneye.pot Updates source line references for translation strings due to code movement.

Comment thread motioneye/config.py
Comment thread motioneye/config.py
Comment thread motioneye/config.py Outdated
Backup and restore only those files which are actually generated by motionEye, and reasonable to restore: motion.conf, camera-*.conf, and prefs.json. uploadservices.json contains mostly redundant overridden info (dedicated cleanup needed), and refresh + access tokens for Google and Dropbox OAuth2. Restoring expired or revoked tokens does not make sense. If any is invalid, the OAuth2 authorization loop needs to be done in any case. All other files need to be created on console, manually or via motioneye_init. They do not necessarily need to be writable by the daemon, and should hence be consequently excluded.

In case the config path contains more than 100 files, it was assumed to be "a system-wide config directory", and selective motion.conf + camera-*.conf were added to the backup already, indicating that these were the major files this feature was intended for. This commit aligns things by doing selective backup in any case, adding the reasonable/harmless prefs.json (visual preferences for the GUI), and applying the same file selecting for restoring backups.

For easier OS-agnistic filtering, the native Python tarfile module is used instead of the external `tar` command.

Signed-off-by: MichaIng <micha@dietpi.com>
@MichaIng
Copy link
Copy Markdown
Member Author

@copilot
Please add tests for backup and restore, which also verify that only files matching motion.conf, camera-*.conf, and prefs.json are added to and restored from the tarball into the motionEye config directory.

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

@copilot
Please add tests for backup and restore, which also verify that only files matching motion.conf, camera-*.conf, and prefs.json are added to and restored from the tarb...

Added tests/test_config.py in commit 9cc166c with 13 tests covering both backup() and restore():

Backup tests verify:

  • motion.conf, camera-*.conf, and prefs.json are included
  • Other files (uploadservices.json, secrets.json, motioneye.conf) are excluded
  • prefs.json is silently omitted when absent
  • An empty config dir produces an empty tarball

Restore tests verify:

  • motion.conf, camera-*.conf, and prefs.json are extracted with correct content
  • Non-matching files (uploadservices.json, secrets.json, motioneye.conf) in the tarball are ignored
  • Tarball entries prefixed with ./ (GNU tar convention) are accepted
  • Return value is {'reboot': False} when ENABLE_REBOOT is off
  • Invalid (non-tarball) data returns None

Comment thread tests/test_config.py Outdated
Comment thread tests/test_config.py Outdated
@Marijn0
Copy link
Copy Markdown
Member

Marijn0 commented May 22, 2026

Motion/privacy mask files probably also need to be backed up like before? Maybe monitor scripts and action button scripts too, although I’m less sure about those because restoring executable scripts from a backup may have security implications?

(I’m currently not able to test these changes until tomorrow.) But I think it looks good. 👍

@MichaIng
Copy link
Copy Markdown
Member Author

@copilot
Please fix the failed tests. IMO, _conf_files() can be removed, and replaced with direct os.listdir(self.conf_dir) calls. There is really not much benefit to wrap this into a function.

@MichaIng
Copy link
Copy Markdown
Member Author

@Marijn0
Mask files is a good point. Monitor and action button scripts are explicitly excluded, as they cannot be added with the GUI either, and we do not want someone with GUI access to implicitly be able to execute any command by restoring custom scripts.

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

@copilot
Please fix the failed tests. IMO, _conf_files() can be removed, and replaced with direct os.listdir(self.conf_dir) calls. There is really not much benefit to wrap this ...

Fixed in commit a9a7142. The CI failure was a mypy error — conf_dir set in setUpClass wasn't visible to the type checker without an explicit class-level annotation. Added conf_dir: str to both test classes, and removed the _conf_files() helper, replacing all usages with inline os.listdir(self.conf_dir) calls.

@MichaIng
Copy link
Copy Markdown
Member Author

@copilot
As mentioned by Marijn0 above, please add mask files to the patterns of files to backup and restore. If I see it correctly, it is mask_*.pgm: https://github.com/motioneye-project/motioneye/blob/enh/backup/motioneye/utils/__init__.py#L457-L465
Complement tests accordingly.

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

@copilot
As mentioned by Marijn0 above, please add mask files to the patterns of files to backup and restore. If I see it correctly, it is mask_*.pgm: https://github.com/motione...

Done in commit 74cf97c. Added mask_*.pgm to the backup glob patterns and the restore allowlist in config.py, plus three new tests: test_backup_includes_mask_files, test_backup_omits_missing_mask_files, and test_restore_extracts_mask_files. All 16 tests pass.

Signed-off-by: MichaIng <micha@dietpi.com>
@MichaIng MichaIng merged commit e129f9b into dev May 22, 2026
20 checks passed
@MichaIng MichaIng deleted the enh/backup branch May 22, 2026 19:38
@MichaIng MichaIng linked an issue May 23, 2026 that may be closed by this pull request
@MichaIng MichaIng mentioned this pull request May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

task.pickle in backups

4 participants