Cyberpanel delete all files and directors public_html

Ok, so to clarify, when chmod permissions are:

  • 644 (Owner can Read/write, groups and others can Read) - backups work fine
  • 600 (Owner can Read/Write) - backups delete files

It seems like Backups should work for any permission level, so that you can set them as required for your site’s needs.

But this brings up a more general issue - what should default permissions be?

It seems to me that 600 should be the default for all permissions, no? Why would other users have read access to another site?

In fact, why is it 600 rather than 700 (allow execute permissions as well)? Is execute never something that needs to be done? Or is it a great security issue and execute permissions should only be granted manually to specific files?

  • 644 (Owner can Read/write, groups and others can Read) - backups work fine
  • 600 (Owner can Read/Write) - backups delete files
    600 fine if the file is owned by the website user.

It only fails if the website user cant read the file ex: -rw------- 1 root root

The defaul permition can be like it is, is the standar and needed if you are editing files during development, and if the website allows to upload files the folders sould also allow wite.

If the backup make the backup and discard the files it coulnt read is aceptable(may give warning)

The issue is not the backup fail, is the deleted public_html folder.

I’m searching in cyberpanel source code and didnt found a reason to delete files the only os.delete i see is for pid files. unless the pidfile is notset i cant figureout why it deletes a folder.

1 Like

What we got to know with few users on debugging about this issue is that, the public_html folder gets deleted when the server has low disk space left.

And please make sure you are on the latest version of cyberpanel (minor changes doesn’t get new version number and hence its recommended to run the cyberpanel upgrade command whenever possible, maybe atleast once a month or twice)

I’m pretty sure he has confirmed that there aren’t disk space issues and has latest versions. It is quite possible that there’s an issue with backups when permissions are changed.

1 Like

forgive me…
but cyberpanel updated is not about the version

there is sub-version sometimes submitted by developer without changing the version

so your 2.1.2 with last upgrade february
will different with
2.1.2 with upgrade/installed today (if any “hidden” update between feb and today)

so i agree with

but… sometimes… new “hidden” upgrade bring some glitch and make old bug comming back or new bug :slight_smile:

1 Like

Please see what they already said - its a fresh install.

Though, this is yet another example of why CyberPanel needs better version control.

Good information here.

But why would someone keep a root owner file under a user directory?

Because the files inside there should be owned by user.

I will test this out though.

I’ve created a file as root but nothing got deleted

root@ip-172-26-15-87:/home/incbackus.cyberpanel.net/public_html# ls -la
total 228
drwxr-x---  5 incba4896 nogroup    4096 Mar 18 08:47 .
drwx--x--x  8 incba4896 incba4896  4096 Mar 16 14:43 ..
-rw-r--r--  1 root      root          0 Mar 18 08:47 hello.txt
-rw-r--r--  1 incba4896 incba4896   725 Mar 16 06:42 index.html
-rw-r--r--  1 incba4896 incba4896   405 Mar 16 06:44 index.php

hello.txt is file owned and created by root

However, when the file is like this

root@ip-172-26-15-87:/home/incbackus.cyberpanel.net/public_html# ls -la
total 228
drwxr-x---  5 incba4896 nogroup    4096 Mar 18 08:47 .
drwx--x--x  8 incba4896 incba4896  4096 Mar 16 14:43 ..
-rw-------  1 root      root          0 Mar 18 08:47 hello.txt
-rw-r--r--  1 incba4896 incba4896   725 Mar 16 06:42 index.html

public_html is now gone.

Its good finding, I will dig more and revert.

1 Like

I’ve pushed a commit to resolve this. Although to my eyes there was nothing wrong as I checked this thing multiple times.

It was a simple copy linux command you everyone of you can see here → bug fix: https://community.cyberpanel.net/t/cyberpanel-delete-all-fil… · usmannasir/cyberpanel@e6ed509 · GitHub (on line 372).

I’ve replaced it with copytree function from shutil and it seems to resolve the issue.

Let me know.

Thanks.

2 Likes

I understand your point but the most important question in my point of view is “why should it delete an entire folder BECAUSE there is a root owned file in the directory”

Thanks for the commit, i will try to keep digging because this solution for me is more of an work around because probably(i didnt test it) if in this code

if ProcessUtilities.normalExecutioner(command) == 0:
                raise BaseException(f'Failed to run {command}.')

if it is replaced with one

ProcessUtilities.normalExecutioner(command) 

It will work ok, probably.

The real issue here, i think, is why when we raise the exception it ends ups deleting the public_html folder. Without finding out the reason soon or later the bug will appear again.

Another thing we should be considering is that with the new commit if the user get out of disk space the copy tree will be incomplete, but the user don’t get that information, for the user it will look like the backup went all ok.

Or in the worst scenario, copy_tree will raise an exception and public_html will be deleted. From the thread comments, i think that was the reason copy_tree was commented and replaced with a “cp command”

dear mr usman…

is this bug mean… all cyberpanel’s user must upgrade
or only for people that had this bug ?

Well I am being honest. Even I can not figure out why a simple CP command will result in deletion of whole public_html directory.

As far as I remember I removed copytree because of security concern and had to run the command as user, but now this whole function is run through user so it can use copytree as well.

And when the backup fails it will for sure give an error to the user as it will raise an exception.

Also the normal execution won’t send command to lscpd. I beleive there can be something going on in lscpd binary?

I will talk to david about this, but this should fix the problem because copytree works fine for long time before we changed it due to security reasons.

If you find anything else, feel free to let me know.

yes. for sure, its important.

1 Like

If its lscpd related that bad.

I believe (due to my experience) that this could be a cleanup related due to backup failure. The temporary backup folder is not deleted when the exception is raised, so, probalby when the exception is raised the code that grab it is not being the right one. I’m trying to create a development environment so that i can do debuging and see the flow of the code.

1 Like

ah yes. I will call the cleanup code. Rest everything should be ok.

@ricardojds If you figure out a development environment, can you please make a post about how to do so? I have started tinkering with adding rclone support to incremental backups and would love to follow the code l, but couldn’t figure out how to do so. So I am now just printing various error messages in different places to figure out how it works.

Usman, if you could help with this, it would greatly help us contribute to the code - be it bug fixes or new features.

PM me your email, I will share with you the video.

I just found the line of code that deletes the folder, this is the stranger thing i found ever, and means that is could be a huge bug somewere

print(f"Aborted, {str(msg)}.[365] [5009]")

if you comment this line it wont delete the public_html folder does this make any sense to someone?

1 Like

Just to get this to an almoust end

if we replace this line

raise BaseException(f'Failed to run {command}.')

with this

raise BaseException(f'Failed to run .')

it doesn’t delete the folder

So the issue is because we are concatening the os command to string, and somewhere it get some other component bug on the message piping probably in lscpd or lswsgi that runs a command that grabs part of the cp command.

1 Like