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

CleanAfterReboot #4222

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

CleanAfterReboot #4222

wants to merge 9 commits into from

Conversation

Jkkoi
Copy link

@Jkkoi Jkkoi commented Sep 8, 2024

The PR provides a menu action to clean FileRootPath of a box after the machine reboot.

@DavidXanatos
Copy link
Member

The code does not compile, see failed CI runs.
Further more, I'm not sure if a feature like this implemented this way is desirable.
Not all sandboxes may be intended to be deleted, some may contain persistent software installations, etc.
This feature also bypasses the snapshot mechanism and just deletes all.
Imho it should be implemented differently allow per box granularity and the ability to roll back to last snapshot instead of deleting all.

@DavidXanatos
Copy link
Member

if(MoveFileExW(fileRoot.toStdWString().c_str(),NULL,4)==0){
would be better as it would work with unicode paths,
but that does not address my initial concerns, imho such a feature should be implemented in sandman in a way that basically triggers the normal delete box content code path for selected boxes after reboot.
and not using windows delayed file operations

@Jkkoi
Copy link
Author

Jkkoi commented Sep 8, 2024

This will ensure that the deletion operation gets done reliably, especially if Sandman doesn't start itself at boot time.

@Jkkoi Jkkoi force-pushed the master branch 3 times, most recently from a722a4c to 3b0d8c7 Compare September 16, 2024 11:12
if(QMessageBox::question(this, tr("Sandboxie-Plus"),
tr("Do you want to use Windows Delecting Feature(MoveFileEx)?Or we'll delete the box when Sandman.exe called by userinit.exe."),
QMessageBox::Yes, QMessageBox::No)==QMessageBox::Yes){
if(MoveFileExW(fileRoot.toStdString().c_str(),NULL,4)==0){
Copy link
Contributor

Choose a reason for hiding this comment

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

MoveFileExW->MoveFileExA

@Jkkoi
Copy link
Author

Jkkoi commented Sep 17, 2024

@DavidXanatos I have provided another clean option.

@offhub
Copy link
Collaborator

offhub commented Sep 17, 2024

It might be better to check for a 'DON'T-USE.TXT' file in both the sandbox directory and its parent directory. If the files are not found, the operation (MoveFileEx) should be aborted.

sbiePR4222-dontuse

auto pBoxEx = pBox.objectCast<CSandBoxPlus>();
QString fileRoot=pBoxEx->GetFileRoot();
if(QMessageBox::question(this, tr("Sandboxie-Plus"),
tr("Do you want to make the box cleaned after machine reboot?"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
tr("Do you want to make the box cleaned after machine reboot?"),
tr("Do you want the box to be cleaned after the machine reboots?"),

tr("Do you want to make the box cleaned after machine reboot?"),
QMessageBox::Yes, QMessageBox::No)==QMessageBox::Yes){
if(QMessageBox::question(this, tr("Sandboxie-Plus"),
tr("Do you want to use Windows Delecting Feature(MoveFileEx)?Or we'll delete the box when Sandman.exe called by userinit.exe."),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
tr("Do you want to use Windows Delecting Feature(MoveFileEx)?Or we'll delete the box when Sandman.exe called by userinit.exe."),
tr("Do you want to use the Windows deleting function (MoveFileEx), or delete the box when SandMan.exe starts?"),

QMessageBox::Yes, QMessageBox::No)==QMessageBox::Yes){
if(MoveFileExA(fileRoot.toStdString().c_str(),NULL,4)==0){
QMessageBox::warning(this, tr("Sandboxie-Plus"),
tr("The operation failed,please make sure that Sandman has admin privliage.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
tr("The operation failed,please make sure that Sandman has admin privliage.")
tr("The operation failed. Please make sure that SandMan has administrative privileges.")

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.

4 participants