General bugs and sanity testing of secret unix commands

[tegefaulkes]

Specification

I've been going over the new secrets commands to see if they function as expected. I'll be outlining problems I find here. I'm not considering any localpath to vault crossover for now.

• Make sure all errors exit with the code that matches the unix command.

General questions

1. How should we format the errors. I want to have fs style errors match the unix commands way of doing it. such as rm: cannot remove 'nodir': No such file or directory. However what if the error is a missing vault? Should we format that better than we are right now? Keep it more inline with the fs style error formatting?
2. How do we want to handle ../ in paths that point outside of the root tree for the vault. Writing treats it as in the root directory. But ls will append the ../ paths to the file names. The ls behaviour needs to be changed but is the write behaviour allowed?

write

expectations

• 1. no args gives help text. vault and path must be provided.
• 2. existing vault with no path should give EISDIR type error. before allowing input
• 3. non-existing vault with no path errors should give a ErrorVaultsVaultUndefined error. Before providing input.
• 4. writing to an existing secret should work and overwrite the file.
• 5. Writing to a new file should create it.
• 6. Writing without a newline should not add it.
• 7. Providing multiple args should error with help - Its writing the first path provided.
• 8. We should properly handle ../ paths. - Works but treats it as the root directory. Do we allow this or explicitly not allow the path?

rm

• 1. no args gives help text. vault and path must be provided.
• 2. Specifying existing vaultname should... - How do we want to handle this? We want to treat it as the root directory so we have 3 options. Don't allow it at all with a isRootDir error or something, Allow it but special case to delete the vault if empty of recursive set. Or allow but special case to just delete all root contents.
• 3. Specifying a not existing vault name should error with a vaults ErrorVaultsVaultUndefined error. - Right now it's giving a EPERM cannot remove vault root error message. (addressed in Polykey#838)
• 4. Can remove a single file
• 5. Can remove multiple files
• 6. Can remove multiple files across vaults
• 7. remove from non existing dir should give formatted ENOENT - gives error but not the expected formatting (addressed in Polykey-CLI#320)
• 8. remove empty dir without complaint.
• 9. remove full dir should error with formatted is a directory - Got error but not expected format. (addressed in Polykey-CLI#320)
• 10. remove full dir with recursive.
• 11. Multiple paths should handle non existing path with formatted error (addressed in Polykey-CLI#320 and Polykey#838)
• 12. Multiple paths should handle directory with formatted error. (addressed in Polykey-CLI#320 and Polykey#838)

edit

• 1. no args gives help text
• 2. with exsting vault should give formatted EISDIR error - getting a bad error TypeError: The "path" argument must be of type string. Received undefined (addressed in Polykey#838)
• 3. with non existing vault should give a vaults undefined error - getting a bad error TypeError: The "path" argument must be of type string. Received undefined (addressed in Polykey#838)
• 4. create new file
• 5. existing file includes existing contents and is modified
• 6. create new file in directory
• 7. create new file in non existing directory should error before editing - opens editor and fails with undefined.
• 8. Keeping the editor open will result in a websocket connection timeout after a while. This needs to be fixed. (addressed in Polykey-CLI#320)

cat

• 1. no args gives help text
• 2. with exsting vault should give EISDIR formatted error
• 3. with non existing vault should give vaults undefined error - Should we treat this as a non existing path?
• 4. non existing file should give formatted ENOENT error
• 5. Existing file should output contents
• 6. Multiple files should output contents concantiated together
• 7. Files across vaults should work like multiple files.
• 8. file in directory should output contents
• 9. directory should give EISDIR formatted error
• 10. non exsting file in non existing directory
• 11. multiple paths include non existing path
• 12. multiple paths include directory

ls

I'm not sure we added multiple path support to ls yet.

• 1. No args gives help text.
• 2. existing vault should list vaults contents
• 3. non existing vault should give vaults undefined error. - should we treat this as a non existing path?
• 4. non existing path should give ENOENT error - theres an error but it's badly formatted
• 5. exiting file should just list itself - giving a not a directory error.
• 6. multiple files should list themselves - giving a not a directory error
• 7. listing should work across vaults - only list the first argument
• 8. directory should list contents
• 9. non existing path inside non existing directory should ENOENT formatted error - gives a badly formatted error
• 10. handles multiple paths with non existing path and directory - multiple paths not supported

This still needs work to support multiple paths.

mkdir

• Need to fill this in.

Be sure to add onto the lists if you think of any checks to do.

Additional context

Tasks

1. Run through the list and check the problems
2. Group problems into child-issues. Use your best judgement here but per command is probably the best way to break it down unless the problem applies to multiple commands at once.

[linear]

ENG-451 General bugs and sanity testing of secret unix commands

[aryanjassal]

Specifying existing vaultname should... - How do we want to handle this? We want to treat it as the root directory so we have 3 options. Don't allow it at all with a isRootDir error or something, Allow it but special case to delete the vault if empty of recursive set. Or allow but special case to just delete all root contents.

The VaultOps.deleteSecret throws an error with code EINVAL for invalid operations, such as attempt to remove the vault root directory. For now, I am handling this error as if handling removal of a write-protected directory. It would throw something like this: rm: cannot remove '/': Permission denied.

This probably needs a review later on, but I think is a sufficient solution for the time being, and also perfectly aligns with how Unix commands work.

[CMCDragonkai]

I think we don't actually need to follow unix commands on the error reporting style. But we can still re-use our own ExceptionName: description - message style, however given that we prefer a "single error" in cases where you have multiple errors, you want to get an extended version of AggregateError. I forgot if js-error has a version of this, or maybe we just create own own aggregate error. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError

[aryanjassal]

I think we don't actually need to follow unix commands on the error reporting style. But we can still re-use our own ExceptionName: description - message style, however given that we prefer a "single error" in cases where you have multiple errors, you want to get an extended version of AggregateError. I forgot if js-error has a version of this, or maybe we just create own own aggregate error. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError

What about providing stderr feedback on streamed commands? For example, the secrets mkdir command takes in multiple file paths and performs operations on them. As soon as it completes an operation, it returns the result to the client. If it is an error, then it is immediately displayed in stderr, and a final error is thrown at the end signifying something went wrong with one or more operations.

What is proposed here? Should the stderr feedback be completely removed and only the final error should contain the relevant information (which I think is bad TUI practice), or also send the stderr feedback but at the end, create an aggregate of all failing operations and create a new aggregate error with that? I'd say this approach has the best of both worlds as the user gets immediate feedback on what failed in their command, and the final error is a summary of all the failures like how you wanted.