Seagate / ToolBin

All the great tools we have for the field.
126 stars 31 forks source link

please make it visible that --confirm this-will-erase-data does not really erase data #24

Closed LocoLati closed 1 year ago

LocoLati commented 2 years ago

Please describe the feature/improvement you would like to see added.

SeaChest_Lite: Latest version.

When using a so called "data destructive command" with the command line argument "--confirm this-will-erase-data" I expect that there will be at least some data erased.
But when using "--setSectorSize" to change the sector size from 512e to 4Kn for the first time it happened that I made up my mind and changed back from 4Kn to 512e and voila: ALL my data was still there! My suggestion: Please change the words "this-will-erase-data" to something else reflecting the fact that this command does not erase anything but just makes it inaccessible sometimes - maybe relying only that the OS will superpose another file system structure.

Thank You

PS: This is my first suggestion here, maybe it is not the right place. please redirect it if necessary. Thanks

Please explain why you think adding this feature/improvement is important.

A user not being aware that this command is not really destructive may run into problems because confidential data may be unwantedly exposed. So that I feel there should be a clear warning.

vonericsen commented 2 years ago

Hi @LocoLati, This is something we can work on. Sometimes the features implemented from the standard are not always clear if a data erase will happen or if previously written data will still be accessible. Often, the spec leaves some things like this up to the device manufacturer to decide and this can change between firmware versions and products.

I made a change in the latest version (on Seagate.com, but not pushed on this repo yet) that will erase the boot sector when doing the sector size change, so it will erase some data, but it will not erase the entire disk. The purpose of this small erase was to overwrite the fake MBR record put at the beginning of GPT disks to be backwards compatible with older OSs. When this MBR is read in Windows, the drive cannot be reformatted which was causing some problems for users.

In much older versions we had multiple confirmations depending on the operation, but they were a lot to type and it was suggested we simplify to this. Maybe we need a --confirm this-will-erase-data and a --confirm this-may-erase-data depending on the operation being performed.

Do you think adding some text to the help for each operation to identify which operations will erase vs may erase would be helpful in addition to the separate confirmations?

LocoLati commented 2 years ago

Hi @vonericsen

I am happy that my little note arrived at the obviously right place ;-)

Your suggestion to use --confirm this-may-erase-data will do the job. At least for me when I read what I have written before pressing Enter. If you have to deal with many different drives with different methods and times erasing data then your new wording will definitely help to be aware of when data will be erased and when not. That may even save jobs ;-)

BTW IMHO working with a single drive it may make sense to ask and ask again. But if you have to convert many many of these drives even the 30 seconds countdown built for safety makes edgy: to short to go for coffee break and much to long just to watch.

--confirm yes-i-have-read-the-instructions-at-least-three(3)times-so-please-dont-ask-again-just-do-it

do you like that parameter?

Best regards and have a nice day ;-)

vonericsen commented 2 years ago

Hi @LocoLati,

I pushed a change to support "this-may-erase-data" in openSeaChest and it will be pulled into our next SeaChest build as well.

IMHO working with a single drive it may make sense to ask and ask again. But if you have to convert many many of these drives even the 30 seconds countdown built for safety makes edgy: to short to go for coffee break and much to long just to watch.

--confirm yes-i-have-read-the-instructions-at-least-three(3)times-so-please-dont-ask-again-just-do-it

do you like that parameter?

This is an interesting idea. I will have to run it by a few people to see what they think and how it will impact the tools we send out to some of our customers. We have some that we send out on a USB drive that boots into tinycore linux and runs a predefined sequence on matching devices....we rarely know how many are attached to the system in this kind of use-case...could be a handful or a few hundred to a thousand in some configurations I've seen.

Overall I think the current confirmation is really good since it can work with -d all, -d /dev/sg1, and -d /dev/sg1 -d /dev/sg4 -d /dev/sg7 ..... situations and it can work in scripted configurations as well. I think having the new separate confirmation is also clear on the operations that will erase versus those that might depending on what the firmware of the device decides to do.

vonericsen commented 1 year ago

In the latest tools updated to this repo (and I believe the previous release), we added the possible data erase confirmation as well as another warning for low-level reformatting (fast format, depopulate, etc) that was also needed.

I'm closing this for now, but please feel free to reopen this if you think we have more work to do on this issue.