add filestatus command

[endorama]

Closes #460

Add filestatus command, reporting if the file is in encrypted or unencrypted state.

I reused ensureNoMetadata logic, thus the command would return 0 when the file is not encrypted and 203 when the file is encrypted (respecting the codes.FileAlreadyEncrypted error code)

It does not output the error message as returned by ensureNoMetadata, preferring a simpler output: "File is encrypted" or "File is unencrypted".

As I'm not so fond on sops internals, I'm sure there are multiple things to review, I'll gladly update the code to reflect any suggestion.

[codecov-io]

Codecov Report

Merging #545 into develop will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #545   +/-   ##
========================================
  Coverage    36.46%   36.46%           
========================================
  Files           20       20           
  Lines         2863     2863           
========================================
  Hits          1044     1044           
  Misses        1725     1725           
  Partials        94       94

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4b5b7ae...0fba7e1. Read the comment docs.

[kowalczykp]

Any update on that? It would be a really nice feature

[mazdack]

any update? would be really nice to have this command

[autrilla]

No updates. The review comments haven't been addressed in this PR, plus there's merge conflicts. We'd be happy to review a PR that builds on this one an handles the review comments.

[endorama]

@autrilla May you let me know your opinion on https://github.com/mozilla/sops/pull/545#discussion_r339676101?

I was waiting on a reply there, other concerns should have been addressed as requested.

[autrilla]

@autrilla May you let me know your opinion on #545 (comment)?

It wouldn't take a lot of work to have a slightly better check by doing what jvehent suggests, e.g. ensuring there's a mac field. Bonus points for reusing this better check in the place where we already check if a file is encrypted :)

[codecov-commenter]

Codecov Report

Merging #545 into develop will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #545   +/-   ##
========================================
  Coverage    36.44%   36.44%           
========================================
  Files           22       22           
  Lines         3205     3205           
========================================
  Hits          1168     1168           
  Misses        1918     1918           
  Partials       119      119           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4bd640e...ff0888f. Read the comment docs.

[endorama]

@autrilla I resolved conflicts and rebased onto latest develop so this should be now safe to merge again.

I started working on how to improve the check and I found out that using an implementation of EncryptedFileLoader.LoadEncryptedFile(in []byte) (Tree, error) on a non-encrypted file results in a MetadataNotFound error. Would this be ok as a deeper test? It feels "right" from my point of view. Is it enough or once metadata are found should it perform some other check?

[autrilla]

@autrilla I resolved conflicts and rebased onto latest develop so this should be now safe to merge again.

I started working on how to improve the check and I found out that using an implementation of EncryptedFileLoader.LoadEncryptedFile(in []byte) (Tree, error) on a non-encrypted file results in a MetadataNotFound error. Would this be ok as a deeper test? It feels "right" from my point of view. Is it enough or once metadata are found should it perform some other check?

Trying to load the file sounds good to me.

[onedr0p]

@autrilla any status here, or are you waiting on the author to rebase this PR again? It seems like he addressed the issues before but now there's conflicts again. What's the hold up?

[onedr0p]

Bump ❤️

[autrilla]

Not all the issues are addressed. In particular, I asked for a better test that the file is encrypted. It's also missing documentation and tests, and as you mention, there's conflicts.

[endorama]

Hello,

I have redone the implementation.
The check is performed in the function cfs and:

• tries to load the encrypted file
• if it fails, report as not encrypted
• if encryption works, checks for version and mac keys to be present

filestatus subcommand now output JSON as requested ({"encrypted": true|false}).

I rebased onto master, so I'm not sure why it's still complaining about the conflict.

@autrilla is this implementation ok? I also added some tests for it. The cfs function is private in the filestatus package, let me know where is the most appropriate place to put it into so it can be reused in other areas as well.

Thanks and sorry for the long wait!

[Pehesi97]

How is the progress in this PR? Do you need any help?

[aDingil]

@ajvb

[ggordan]

Hi @ajvb @autrilla @jvehent, I see that roughly a year ago the milestone for this issue was changed to v3.8.0.

Would that suggest that this PR is "complete" meaning that when v3.8.0 is released, this will be included, or are there still things that need to be addressed for the maintainers to be happy to merge this? If the answer is no, can we get a indication of what is needed, I'd be more than happy to help out.

Subsequently, is there a rough estimate on when v3.8.0 is expected to land?

Thanks, Gordan

[felixfontein]

@endorama would you mind rebasing another time and signing off your commits? Would be great if we could finally get this merged :)

[davinkevin]

Can't wait to use it!