Add File Shred command #513
Conversation
sfc-gh-iyashchyshyn
requested review from
sfc-gh-ikryvanos and
sfc-gh-mwalas
|
|
||
| // ShredRequest is a request to perform Shred operation on a specific file | ||
| message ShredRequest { | ||
| string filename = 1; |
There was a problem hiding this comment.
I guess this should be absolute path? Should we add a comment?
Btw it is a shame that this prot o file talks about filepath as filename everywhere. https://unix.stackexchange.com/questions/146895/filename-and-pathname-of-a-file
But since local consistency matters too, I guess it is better to keep it as filename.
|
|
||
| args = append(args, req.Filename) | ||
|
|
||
| // we want to fail as soon as possible, if the file is unreadable, opened by someone else or something else happens |
There was a problem hiding this comment.
Was there any alternative you considered here other than attempting shredding and returning error as Internal? I am trying to understand why this comment needs to be in place.
I think what you do here is reasonable, might be little weird that we return "Internal" for filenames that are not found, but we shouldn't go into business of parsing linux commands output(s).
There was a problem hiding this comment.
Is there a way to reasonably extend the localfile_test.go to cover shredding with some test case? (I guess some test on zero-ing without removal is something we could do)
| // listed in one of the PATH's components, it will continue its operation and only return an error in case | ||
| // no paths were readable and/or executable couldn't be found in either of them. | ||
| // Note that it does not check whether a file is actually executable by current user. | ||
| func Which(exeName string) (string, error) { |
There was a problem hiding this comment.
For ansible, the choice so far was to hard-code the bin
do you expect us to have different shredding binary specified per user, hence we start to use which like approach? If so, we should probably sudo to the user first before resolving path?
(not sure if we need that, and if so we could just start with binding to /usr/bin/shred?)
WIP
Adds
shredsub-command tofilecommand with support for zeroing-pass, file-removal and force-permissions-change flags.