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

Incorporate Unix Commands to Secrets Subcommand - pk secrets * #32

Closed
CMCDragonkai opened this issue Aug 30, 2021 · 18 comments
Closed

Incorporate Unix Commands to Secrets Subcommand - pk secrets * #32

CMCDragonkai opened this issue Aug 30, 2021 · 18 comments
Assignees
Labels
design Requires design (architecture, protocol, specification and task list requires further work) development Standard development enhancement New feature or request r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Aug 30, 2021

Specification

Standard unix commands include things like cp, mv and ls.

This allows users to interact with secret vaults as if they were real filesystems. (And they sort of are).

Imagine (IN ORDER OF PRIORITY):

# these 2 might be merged into 1 command just `write` is sufficient
pk secrets read vault1:/a/b
pk secrets write v1:/abc

pk secrets cat vault1:/a/b
pk secrets touch vault1:/a/b
pk secrets ls vault1
pk secrets mkdir vault1:/c
pk secrets rm vault1:/a/b
pk secrets mv vault1:/a/b vault1:/a/c
pk secrets cp vault1:/a/b vault1:/a/c
pk secrets ln vault1:/a/b vault1:/d/e

# this one is special
pk secrets env

# this one is also special
pk secrets ed vault1:/a/b

# these are less important
pk secrets head vault1:/a/b
pk secrets tail vault1:/a/b
pk secrets find vault1
pk secrets grep vault1:/a/b
pk secrets sed vault1:/a/b

I believe some of these commands were already implemented before in the old PK codebase. You have things like pk secrets create and pk secrets delete.

»» ~/Projects/js-polykey/src/bin/secrets
 ♖ tree .                                                                                                          (client-refactoring) pts/5 14:47:05
.
├── create.ts
├── delete.ts
├── dir.ts
├── edit.ts
├── env.ts
├── get.ts
├── index.ts
├── list.ts
├── mkdir.ts
├── rename.ts
└── update.ts

0 directories, 11 files

I believe there's a problem with doing this. We are reinventing the wheel, and we'll never cover all the commands that Unix already has.

This has the benefit of reusing context that developers already know and remember when interacting with a Unix shell.

But how do we do this without having to rewrite all the code? Luckily it seems someone has already done this.

See: https://github.com/shelljs/shelljs

It has implemented all the major Unix shell commands in raw JS.

The only problem that their command source code currently directly imports the native Node fs:

https://github.com/shelljs/shelljs/blob/79ae14d30d7ce4064de05d41c7889885326b6754/src/ls.js#L2

If we would want to use the shelljs library, we would need to globally mock the fs as described here: shelljs/shelljs#747 (comment)

However that may be dangerous if that leaks into other places of the FS.

There is another alternative: https://github.com/dthree/cash. The difference between the 2 are: https://github.com/dthree/cash#doesnt-shelljs-do-this However I think it's even less integratable compared to shelljs.

So it seems we would need to "extract" the command code from shelljs and place it into PK directly and thus enable us to change the fs object to our Vault EFS.

Note that we do not need all shell commands, just the major ones that relate to files, and doesn't change cwd context since we don't use that. Nothing that changes permissions is relevant to us. Process control is also not relevant.

One major difference is that our commands will have to traverse both vault filesystems and the real filesystem. For example pk secrets mv vault1:/a ./a which has to take a file from a vault to the real fs. The functionality to achieve this will also end up being used to do commands between vaults. Because the efs.mv won't work between EFS instances anyway.

Additional context

Sub issues

Tasks

  1. ...
  2. ...
  3. ...
@CMCDragonkai CMCDragonkai added the development Standard development label Aug 30, 2021
@CMCDragonkai CMCDragonkai changed the title Incorporate Standard Unix Commands to Secret Vaults Incorporate Unix Commands to Secrets Subcommand - pk secrets * Aug 30, 2021
@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Sep 2, 2021

With regards to pk secrets cat and "writing a file" to a secret vault. While the cat command can be used to read out a secret, what's more difficult is putting back a secret that can be composed with other commands. Here I propose a null-command or a variant of pk secrets that opens a readable file descriptor.

Here are some examples:

# using process redirection, as pk secrets v1:/a would need to take a FD
echo 'somesecret' > >(pk secrets v1:/a)
printf 'somesecret\n' > >(pk secrets v1:/a)
echo 'somesecret' | pk secrets v1:/a

# echo 'somesecret' > pk secrets v1:/a # <- this will not work as pk secrets is a comand, not a filepath

# compare with shell commands like:
echo 'somesecret' > >(cat > abc)
echo 'somesecret' | cat > abc

Note that pk secrets v1:/a has no subcommand under secrets. It would need to open a file descriptor by attempting to read from STDIN. This takes over from the shell command cat > abc which runs cat without a file path argument which causes it to read from STDIN. However is not usable in PK since we aren't running process redirection operators > and < from inside the PK command. That is PK is not a shell. Therefore we run something different:

# starts reading from STDIN immediately
pk secrets v1:/a

As well as the ability to use cat subcommand to output to STDOUT.

# read v1:/abc and pipe it to cat, pretty simple
pk secrets cat v1:/abc | cat

At this point, one might argue that one should enable pk secrets v1:/a to be a writable file descriptor as well. That is both STDIN and STDOUT may be usable.

cat <(pk secrets v1:/abc)

Which would take over from the cat subcommand.

However I'm not sure I really like the usage of pk secrets v1:/abc as a universal file descriptor. It's nice in one way, but it seems such a "default" command to be relatively hard to use and hard to learn.

Consider that we also need another command that one can just use to put a secret in without the need to wire up other commands.

pk secrets put v1:/abc 'abc'
pk secrets write v1:/abc 'abc'
pk secrets print v1:/abc 'abc'

And as we have talked about before, we need 3 ways:

  • Passing as an argument
  • Passing as a file descriptor
  • Passing as via the prompt

I propose a special command for this:

# this will read from STDIN, and write to v1:/abc (like reading into a secret)
pk secrets read v1:/abc

# pipe into a file descriptor
echo 'secretdata' | pk secrets read v1:/abc
pk secrets read v1:/abc <<<'secretdata'
echo 'secretdata' > >(pk secrets read v1:/abc)

So then read is the opposite of cat.

But still one more command that allows one to write a secret in easily. Here are our 3 candidates.

pk secrets put v1:/abc 'abc'
pk secrets write v1:/abc 'abc'
pk secrets print v1:/abc 'abc'

From these 3, I quite like pk secrets write. It's the opposite of read, it actually can function very similarly if pk secrets write v1:/abc was allowed. That is with no arguments, it acquires from the STDIN. In that sense, there's no need for pk secrets read at all then as it is just confusing. So write is probably the correct keyword.

@CMCDragonkai
Copy link
Member Author

Some commands here due to usage over GRPC will involve client streaming or even duplex streaming. Deadlines will be important here. And if a command gets cut in the middle, the data written to EFS should be removed. That might be good idea to per-vault lock to do this. Until of course we can create a "vault"/efs snapshot. The beginnings of a vault/efs snapshot is already done in MatrixAI/js-encryptedfs#49 but it would need to be exposed to the PK vaults system. Perhaps that can help with enabling COW for the vaults MatrixAI/Polykey#172 MatrixAI/Polykey#180.

@CMCDragonkai CMCDragonkai added design Requires design (architecture, protocol, specification and task list requires further work) enhancement New feature or request labels Sep 2, 2021
@CMCDragonkai CMCDragonkai added the epic Big issue with multiple subissues label Mar 11, 2022
@CMCDragonkai
Copy link
Member Author

Now that MatrixAI/Polykey#266 has fixed up vaults related commands, the next step is a proper review of all the secret commands, and hopefully finish up our pk secrets env PR MatrixAI/Polykey#265 so it can be merged.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Jul 24, 2022
@CMCDragonkai CMCDragonkai transferred this issue from MatrixAI/Polykey Oct 18, 2023
@CMCDragonkai
Copy link
Member Author

@CryptoTotalWar this is relevant for your review of PK CLI on Mac.

@CryptoTotalWar
Copy link

Proposal for Streamlining Secrets Creation Using Unix Piping Paradigms

Concept:

Leverage Unix-like command paradigms to streamline the process of creating secrets directly into the vault, bypassing the need to first create a file on the local filesystem. This would enhance the functionality of Polykey by allowing direct input of secrets via standard input (stdin), aligning with Unix principles of simplicity and flexibility.

Details:

Instead of the current method where a file must exist beforehand, we could introduce functionality where users can pipe data directly into a Polykey secret path. This approach avoids unnecessary steps and strengthens security by not requiring sensitive data to touch the filesystem.

Suggested Command Enhancement Example:

echo 'your_secret_data' | polykey secrets write vaultName:/path/to/secret

This command would take the output from echo and directly write it to the specified secret path in the vault. It simplifies the workflow, especially in scripting and automation scenarios, where secrets need to be dynamically generated and stored.

Benefits

  • Security: Minimizes risk by avoiding intermediate storage of sensitive data on disk.
  • Efficiency: Reduces steps in the secret creation process, enabling faster and more fluid workflows.
  • Usability: Aligns with common Unix practices, making it intuitive for users familiar with command line interfaces.

I believe this enhancement would significantly improve the user experience by making secret management more seamless and integrated with standard command line operations. It encourages best practices in handling sensitive data.

Copy link
Contributor

I'm adding this to the 1.0.0 project. It needs to be further speced out and broken up into child issues for each command we need to add.

@tegefaulkes
Copy link
Contributor

It would be great if we could just use an off the shelf implementation of these commands such as shelljs mentioned above. But there is too much under the hood stuff going on there to make it viable.

The main problem is that we need to send any local FS contents over the RPC to do anything with it. So we'd need a fs interface that works over the RPC which seems like a really hacky way to handle that. You also mentioned that the solutions like shelljs to do DI for the fs interface so it would be really hacky to implement regardless.

So we'll have to make our own implementations of these commands. And since some of these commands are pretty well featured we'd have to make a simpler version of each. Even something simple like cat has a few options we probably don't want to bother with, not at this stage at least.

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Jul 10, 2024 via email

@tegefaulkes
Copy link
Contributor

I was arguing against using shelljs as a shortcut since it would take more work the shoehorn it in.

@tegefaulkes tegefaulkes changed the title Incorporate Unix Commands to Secrets Subcommand - pk secrets * ASGJIL;LLLLLLLIncorporate Unix Commands to Secrets Subcommand - pk secrets * Jul 16, 2024
@tegefaulkes tegefaulkes changed the title ASGJIL;LLLLLLLIncorporate Unix Commands to Secrets Subcommand - pk secrets * Incorporate Unix Commands to Secrets Subcommand - pk secrets * Jul 16, 2024
Copy link

I tested the pk secrets edit command to understand its current behavior and alignment with our discussions on Unix-like functionality for Polykey. During my tests, I encountered an error when attempting to edit a non-existent secret. Here's the output:

~$ polykey secrets edit my-new-vault:my-new-secret                                                            2:28:12
ErrorPolykeyRemote: Remote error from RPC call
  localHost	::1
  localPort	51127
  remoteHost	::1
  remotePort	50053
  command	vaultsSecretsGet
  timestamp	Fri Jul 26 2024 02:28:50 GMT+0100 (Western European Summer Time)
  cause: ErrorSecretsSecretUndefined: Secret does not exist - Secret with name: my-new-secret does not exist
    cause: {"type":"ErrorEncryptedFSError","data":{"message":"ENOENT: no such file or directory, my-new-secret","timestamp":"2024-07-26T01:28:50.388Z","data":{},"stack":"ErrorEncryptedFSError: ENOENT: no such file or directory, my-new-secret\n    at f._open (/Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2158:117625)\n    at async /Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2158:115345\n    at async Object.maybeCallback (/Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2155:21076)\n    at async /Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2158:120774\n    at async Object.maybeCallback (/Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2155:21076)\n    at async /Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2283:135663\n    at async /Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2246:8712\n    at async withF (/Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:7:9819)\n    at async Object.getSecret (/Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2283:135640)\n    at async /Users/rulerpablo/.nvm/versions/node/v20.8.1/lib/node_modules/polykey-cli/dist/polykey.js:2283:142996","_errno":34,"_code":"ENOENT","_description":"no such file or directory","_syscall":"open"}}
~$                                                                   


I wanted to clarify whether the envisioned pk secrets edit command will automatically handle creating a new secret if it does not exist at the specified secretPath. This behavior would be akin to how traditional text editors operate (creating a new file if it doesn't exist when attempting to open it).

Current Command Behavior:

  • The command fails if the secret does not exist, which seems contrary to the typical Unix file handling we aim to emulate.

Suggested Feature:

  • Allow pk secrets edit to initiate a new secret if the specified path does not exist, similar to opening a new file in an editor. This would streamline the user experience and align it with Unix-like behavior, making it intuitive for users familiar with traditional command-line operations.

Example of Expected Behavior / Functionality

Usage of pk secrets edit

Editing an Existing Secret (currently supported)

If the secret already exists within the specified vault, pk secrets edit will open it in the default editor set in the environment (e.g., nano, vim). Any changes made will be saved back to the secret when the editor is closed.

Example:

pk secrets edit vaultName:existingSecretName

This command opens the existing secret in the default editor for modifications.

Creating a New Secret (NOT currently Supported But NEEDED)

If the secret does not exist at the specified path, pk secrets edit will open a new blank session in the editor. Whatever content you input and save will initiate the creation of the secret at that path within the vault.

Example:

pk secrets edit vaultName:newSecretName

This command will start a new file in the editor under the provided newSecretName in vaultName. Once saved, a new secret is created with the content entered.

Practical Considerations

  • Simplicity: Users can directly create and edit secrets without needing separate commands for creation and editing, simplifying the workflow.
  • Flexibility: Supports both the use cases of modifying existing secrets and creating new ones seamlessly.

Security and Implementation:

  • As discussed, any direct input of secret contents as command arguments should be avoided to prevent leakage. Ideally, the command should open the editor for the user to input the secret data securely, polykey secrets edit already does this but for secrets that have already been created. Also it requires some system config. I made a mention of this here: Polykey-CLI: Polykey Secrets Edit command not working as expected on Mac Os  #187

Next Steps:

  • Could we confirm if this behavior will be integrated into the pk secrets edit command? It would be a significant enhancement for user interactions with Polykey, especially for those who prefer command-line efficiency.

Looking forward to your thoughts and any further updates on this functionality.

Copy link
Contributor

tegefaulkes commented Jul 26, 2024

You're being a little too verbose here. This can be condensed down into two points.

  1. Based on our discussion, we still need a command like secrets edit. On top of this, if the secret we want to edit doesn't exist then we should just create that secret and edit it.
  2. Currently we have a secrets edit command that does this but it will not create a new secret for us. It just just throw an error that the secret doesn't exist. That needs to be changed.

I do think we'll still need an edit command that opens up the user defined editor to edit secrets. I'd like to align that behaviour with what you'd expect using nano or vim where it comes to handling the existence of files. So I'll make a new sub-issue for tracking that.

So yeah, we probably want to keep the secrets edit command for this. We just need to review it's usage and modify it slightly.

@CMCDragonkai
Copy link
Member Author

Generally speaking you rely on an environment variable: $VISUAL then if not set, rely on $EDITOR, and if not set, then you default onto something that would exist on most platforms.

https://unix.stackexchange.com/questions/4859/visual-vs-editor-what-s-the-difference

The "default" depends on some magic.

Here's our list depending on the OS.

  1. Linux - VISUAL, EDITOR, nano, ed - I wouldn't default to vi or vim because it's actually a more advanced editor.
  2. Windows - VISUAL, EDITOR, get the .txt file association first, then default to notepad.exe and wait for it to finish. Windows never had a default terminal text editor.
  3. MacOS - VISUAL, EDITOR, pico, nano

If the defaults cannot be found, you need to error out, indicating that no default editor could be found, and users can instead choose to set VISUAL, EDITOR or pass in the "editor" program with --ed or something.

Furthermore it is important that the temporary being created can only be edited by the same user as the user who ran polykey secrets edit, otherwise it can be intercepted. The file should have a limited umask like 700. Or equivalent for the Windows.

@CMCDragonkai
Copy link
Member Author

@tegefaulkes the spec is missing updating secrets edit|ed command. It needs to be able to open a file that doesn't already exist.

And it needs to be more intelligent about finding the $EDITOR.

Plus I think this is a bit more important than secrets cat. So staging the assignments for the cycles for @aryanjassal will be important to be explicitly explained here in the spec.

Copy link
Contributor

This issue has been made into a project, so this issue is now redundant. Closing.

@aryanjassal aryanjassal closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2024
@CMCDragonkai
Copy link
Member Author

@aryanjassal can you also remove old commands like dir if mkdir is there now. We should be reducing entropy.

@aryanjassal
Copy link
Contributor

@aryanjassal can you also remove old commands like dir if mkdir is there now. We should be reducing entropy.

The dir command actually works slightly different to mkdir. The dir command copies a directory of secrets from the user's system and puts the directory and their contents in the vault. As such, I thought that dir should be removed when cp is implemented instead.

@CMCDragonkai
Copy link
Member Author

Yep we should remove the dir asap and get cp into the situation.

@CMCDragonkai
Copy link
Member Author

I think based on the revaluation of secrets commands project spec, you should start being able to refactor the core architecture of the PK CLI and PK stuff too and not just on the outside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Requires design (architecture, protocol, specification and task list requires further work) development Standard development enhancement New feature or request r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Development

No branches or pull requests

5 participants