secrets edit creates a secret if it doesn't exist

[aryanjassal]

Description

This PR tracks improving the secrets edit command to create a file in the vault if it didn't exist, otherwise edit the file contents as usual.

This iteration will focus on creating a new secret if it didn't exist at the time of editing it. Other features from the issue, including platform agnostic editor search or using RawHandler to stream file contents instead of UnaryHandler will be implemented in future iterations.

Issues Fixed

• Relates to Implement secrets edit command #266
• REF ENG-398

Tasks

• 1. Create file if it doesn't exist in the vault
• 2. Write tests for secrets edit

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[linear]

ENG-358 Implement `secrets ls` command

[aryanjassal]

Based on the discussion we had in #255, for rapid iteration over these commands, do I merge this PR which introduces automatic file creation, or do I keep working on it and implement the platform-specific $EDITOR commands, which would take longer and is currently blocked by me not having access to a Windows or a Mac at this moment?

@CMCDragonkai @tegefaulkes

[linear]

ENG-398 Implement `secrets edit` command

[tegefaulkes]

If we're keeping to the iterative style then we should merge the current fix and move editor changes to a new issue.

There's a little more work to do here however. We need to use the new secret contents serialiser to stream over the secret contents.

[aryanjassal]

There's a little more work to do here however. We need to use the new secret contents serialiser to stream over the secret contents.

Oh, do you mean that we stop using vaultsSecretsGet and instead use the file contents serialiser proposed in MatrixAI/Polykey#785?

[tegefaulkes]

Yep, we nee to use the new serialiser to get the contents but also send the changed file back.

[aryanjassal]

#266 (comment)

[tegefaulkes]

We need to add tests to show functionality of this command. Its actually a problem that there are no tests already but understandable because its kinda hard to test interactive input. You can create a test node or bash script and use that as a psudo editor for testing. The script will do two things, return to the test the contents of the file so we can verify it, but also edit the file with the contents we provide it.

These tests are essential to check for regressions when future changes are made so we really need tests here that cover all the intended functionality of the command. There are two main cases.

1. edit the file and check that the contents are updated. With the current code this would actually fail.
2. Edit a new file and show that it is created with the contents.

[aryanjassal]

Polykey-CLI/src/secrets/CommandEdit.ts

Line 89 in 6c483ad

execSync(`$EDITOR \"${tmpFile}\"`, { stdio: 'inherit' });

Apparently when you invoke environment variables using SVARIABLE in execSync, it doesn't respect the variable value in process.env. For me, while I had set process.env.EDITOR to a script I made for testing the secrets edit command, the execSync kept opening my actual system editor.

However, logging the value of process.env.EDITOR showed me that the value was set correctly. So, when I replaced $EDITOR with process.env.EDITOR, the command started respecting the actual environment variable and using the script correctly.

It is weird, because ExecSync by default uses NodeJS.ProcessEnv for its environment variables, but for some reason, it didn't work with the tests.

[tegefaulkes]

Some small changes are we're good.