Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OctoPrint requires parent folder permissions to restore from backup #4961

Open
1 of 4 tasks
stephen-mw opened this issue Feb 25, 2024 · 5 comments
Open
1 of 4 tasks
Labels
done Done but not yet released improvement Improving functionality, behaviour, UX, ...
Milestone

Comments

@stephen-mw
Copy link

The problem

I have my octoprint root directory at /etc/octoprint. Restoring from a backup fails because the octoprint user doesn't have permission to create files in /etc which is expected behavior:

Uploading backup, this can take a while. Please wait...
 
Restoring from backup...
 
Unpacking backup to /tmp/tmplba213wr...
Unpacked
Renaming /etc/octoprint to /etc/octoprint.bck...
Removing temporary unpacked folder
Error while running restore
Traceback (most recent call last):
  File "/usr/lib/python3.11/shutil.py", line 825, in move
    os.rename(src, real_dst)
PermissionError: [Errno 13] Permission denied: '/etc/octoprint' -> '/etc/octoprint.bck'

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/dist-packages/octoprint/plugins/backup/__init__.py", line 1323, in _restore_backup
    shutil.move(basedir, basedir_backup)
  File "/usr/lib/python3.11/shutil.py", line 841, in move
    copytree(src, real_dst, copy_function=copy_function,
  File "/usr/lib/python3.11/shutil.py", line 561, in copytree
    return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/shutil.py", line 459, in _copytree
    os.makedirs(dst, exist_ok=dirs_exist_ok)
  File "<frozen os>", line 225, in makedirs
PermissionError: [Errno 13] Permission denied: '/etc/octoprint.bck'
 
Restore failed! Check the above output and octoprint.log for reasons as to why.
Removing temporary zip

This following can and should always fail because octoprint should never get write permissions to the parent directory:

PermissionError: [Errno 13] Permission denied: '/etc/octoprint' -> '/etc/octoprint.bck'

One workaround is to put octoprint in a subdirectory where it can have parent permissions such as /etc/octoprint/octoprint.

That has the following drawbacks:

  1. More directories and setup are required for managing permissions.
  2. Octoprint can always write to the parent so nothing else should be stored there.

I'd like to propose the following fix:

  1. When octoprint does a restore it backs everything up to /tmp and then removes the contents of the directory.
  2. Once the contents are removed, the restore zip contents are unzipped directory into the root directory.

Thoughts on these changes? I can start working on it.

Did the issue persist even in safe mode?

Yes, it did persist

If you could not test in safe mode, please state why ("currently printing" is NOT an excuse!)

No response

Version of OctoPrint

octoprint, version 1.9.3

Operating system running OctoPrint

Debian GNU/Linux 12 (bookworm) (rasberry pi 4)

Printer model & used firmware incl. version

No response

Browser and version of browser, operating system running browser

No response

Checklist of files to include below

  • Systeminfo Bundle (always include!)
  • Contents of the JavaScript browser console (always include in cases of issues with the user interface)
  • Screenshots and/or videos showing the problem (always include in case of issues with the user interface)
  • GCODE file with which to reproduce (always include in case of issues with GCODE analysis or printing behaviour)

Additional information & file uploads

octoprint-systeminfo-20240225162317.zip

@github-actions github-actions bot added the triage This issue needs triage label Feb 25, 2024
@jneilliii
Copy link
Contributor

Default behavior of OctoPrint sets the basedir parameters to /home/pi/.octoprint/ and the files are installed in /home/pi/OctoPrint/ following official documentation. If you want to put the install files elsewhere you need to make adjustments to the service files to run OctoPrint as root, which is not a recommended approach for obvious security reasons. I question why you installed into etc though?

@stephen-mw
Copy link
Author

Thank you for the response @jneilliii . I run octoprint as a service on my raspberry pi along with a lot of other services. I believe it's common for linux services to keep their configuration files under /etc/ (such as /etc/nginx, /etc/apache, etc).

There's no octoprint home directory or user on my system for security reasons.

I don't use the octoprint image, rather I install the software myself. Otherwise it works great!

stephen-mw added a commit to stephen-mw/raspberrypi that referenced this issue Feb 27, 2024
- There are some issues when running octoprint in a different directory thatn home.
    - see: OctoPrint/OctoPrint#4961
- Some updates for installing kiwix software.
@foosel foosel modified the milestones: 1.10.0, 1.11.0 Mar 6, 2024
@foosel
Copy link
Member

foosel commented Mar 6, 2024

I'd like to propose the following fix:

  1. When octoprint does a restore it backs everything up to /tmp and then removes the contents of the directory.

  2. Once the contents are removed, the restore zip contents are unzipped directory into the root directory.

Sounds good, would have been my solution as well.

@foosel foosel added improvement Improving functionality, behaviour, UX, ... and removed triage This issue needs triage labels Apr 11, 2024
@foosel
Copy link
Member

foosel commented May 22, 2024

Sounds good, would have been my solution as well.

I stand corrected, that's not an option. The thing is, right now we are doing two pretty atomic move options. We are basically moving the old config dir out of the way (backing it up in the process) and then put the new one back where it was. If we switch that to emptying the current config folder and then copying (or moving) over individual files from the backup, there's WAY more that can go wrong in the process, at which point the user would be left with a complete mess vs a situation where they just have to rename the created .bck config folder and are back in working order to start a new try.

That's too risky.

We need an easy way to roll back, and an atomic as possible restore.

What I could see working is not moving the directory, but rather zipping it up in itself, then in the next step copy over the backup, and if anything goes wrong in that process, delete everything again apart from the backup zip, and unzipping that again in place before removing it. However, that are still a lot of moving parts that are only ever needed in a setup as mentioned here, which is not the default. So switching everyone over to that would mean everyone who doesn't even need that amount of complexity gets a higher risk of restore failure in exchange, which strikes me as unfair. Supporting both options in parallel could be done (check if the base dir can be moved, if that fails it's apparently not owned by the process, at which point you move over to the other approach), but I'm not sure I want to maintain two process variants.

I'll have to mull that over for a bit, and see how bad it really could be if I went that route...

foosel added a commit that referenced this issue May 22, 2024
@foosel
Copy link
Member

foosel commented May 22, 2024

Ok, that wasn't as bad as I thought. If the initial rename of the basedir to the .bck suffix fails, a copy approach as described above will now be attempted. Normally that shouldn't trigger in most installations out there, only if necessary.

Will be part of 1.11.0.

@foosel foosel added the done Done but not yet released label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done Done but not yet released improvement Improving functionality, behaviour, UX, ...
Projects
Status: Done
Development

No branches or pull requests

3 participants