Secret env command

[scottmmorris]

Description

This PR aims to refactor the existing pk secret env command according to the specifications outlined in the Development Environment Usecase. Essentially the command should be able inject environment variable or allow sourcing variables into an existing subshell.

Issues Fixed

• Fixes The pk secrets env command for meeting Development Environment Usecase Polykey-CLI#31

Tasks

1. Ensure that pk secrets env can be used to inject environment variables and run a subprocess
2. Ensure that pk secrets env can be used to source environment variables and output something that can be used as a file to be sourced by a shell
3. Handle export flag
4. Handle globbing options specified in the usecase
5. Handle command options with spaces (may extend to other secret and vault CLI commands)
6. Extending globbing to match documentation here (https://git-scm.com/docs/gitignore)
7. Testing for secret env command executing a subshell
8. Deal with the fact that nodejs doesn't have an exec that can replace the current node process

Final checklist

• Domain specific test (npm test -- tests/bin/secret.test.ts)
• Full tests with npm test
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[scottmmorris]

Ive also changed the naming for pk secrets create to be rpimarily pk secrets put with an alias of create. The main functionality of this command has been added now as well

[scottmmorris]

I've updated the description of this PR to include testing for the launching subshells with injected secrets. At the moment the standard jest testing that I have been writing wouldn't be able to deal with it but there should be a way to test this behaviour with jest. Roger has linked these packages https://github.com/jprichardson/node-suppose & https://github.com/nodejitsu/nexpect as references. I have validated the correct operation by running the command but it will still be better to include automated tests as well.

The other task left for this is optimisation using kexec which I haven't figured out yet. But this will be left until the end because it is an optimization feature.

Also the globbing currently only handles the single globstar *. So cases like vault:* or vault:secretdir/* will both work (getting all files in the base and secretdir directories respectively). Eventually this should be extended to mimic the documentation here (https://git-scm.com/docs/gitignore)

[scottmmorris]

Globbing has now been extended to include both * and ** functionality

[CMCDragonkai]

Which globbing library did you use? Make sure that it is well supported and ideally in typescript.

…

On 7/14/21 4:00 PM, scottmmorris wrote: Globbing has now been extended to include both |*| and |**| functionality — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHPHXXCJUEYFWWXE4VDTXURYRANCNFSM5AIFDI5Q>.

[scottmmorris]

At the moment I wrote my own code to handle globbing. A lot of the globbing libraries I looked at use a cwd to return output which is difficult with the EncryptedFS. This one allows you to supply an fs and is up-to-date but that would require a bit more work in creating a grpc call to handle this and returning the files. Alternatively, this library was another popular one but doesn't accept any other filesystems as far as I am aware of

[scottmmorris]

After trying the first library it seems that EFS is incompatible with the fs implementation specified by fast-glob

No overload matches this call.
  Overload 1 of 2, '(source: string | string[], options: Options & EntryObjectPredicate): Promise<Entry[]>', gave the following error.
    Type 'EncryptedFS' is not assignable to type 'Partial<FileSystemAdapter>'.
      Types of property 'readdirSync' are incompatible.
        Type '(path: PathLike, options?: { encoding: BufferEncoding; withFileTypes?: false; }) => string[]' is not assignable to type 'ReaddirSynchronousMethod'.
          Types of parameters 'options' and 'options' are incompatible.
            Property 'encoding' is missing in type '{ withFileTypes: true; }' but required in type '{ encoding: BufferEncoding; withFileTypes?: false; }'.

[CMCDragonkai]

The fs that can be supplied would be the VFS instance, not the EFS instance. Would fast-glob work in that case? I'm working on EFS extending VFS, so then EFS instance will be a VFS instance anyway.

…

On 7/14/21 4:42 PM, scottmmorris wrote: At the moment I wrote my own code to handle globbing. A lot of the globbing libraries I looked at use a cwd to return output which is difficult with the EncryptedFS. This <https://www.npmjs.com/package/fast-glob> one allows you to supply an fs and is up-to-date but that would require a bit more work in creating a grpc call to handle this and returning the files. Alternatively, this <https://www.npmjs.com/package/glob> one can accept an array of files to glob but is less supported. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHKT7C3BPILQQAG7BJ3TXUWTXANCNFSM5AIFDI5Q>.

[scottmmorris]

After some testing we have opted to introduce the tiny-glob source code into polykey. Have got the source code to work with vfs and have moved it into a utility function. I am creating the new grpc command now which will use this function.

[CMCDragonkai]

We expect some edge case bugs with the globbing since the upstream repo has bugs already in it that hasn't been fixed (https://github.com/terkelg/tiny-glob/issues). But it's not critical so we move ahead.

As for bringing in the globbing code, make sure you're rewriting important parts with TypeScript, as the upstream tiny-glob is written in JS.

[CMCDragonkai]

This requires a rebase on top of client-refactoring given that much has changed.

[CMCDragonkai]

Sorry this was closed, it should have had its target changed to master.

Can you try to edit this @scottmmorris? If not, it should just be reopened again...

isaacs/github#1557 (comment)

Ok I think this PR must be "recreated" then.