secrets edit creates a secret if it doesn't exist

src/secrets/CommandEdit.ts

@@ -1,8 +1,8 @@
 import type PolykeyClient from 'polykey/dist/PolykeyClient';
 import fs from 'fs';
 import path from 'path';
-import * as errors from '../errors';
 import CommandPolykey from '../CommandPolykey';
+import * as errors from '../errors';
 import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
 import * as binParsers from '../utils/parsers';
@@ -24,6 +24,7 @@ class CommandEdit extends CommandPolykey {
     this.action(async (secretPath, options) => {
       const os = await import('os');
       const { execSync } = await import('child_process');
+      const vaultsErrors = await import('polykey/dist/vaults/errors');
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
       );
@@ -44,6 +45,10 @@ class CommandEdit extends CommandPolykey {
       this.exitHandlers.handlers.push(async () => {
         if (pkClient != null) await pkClient.stop();
       });
+
+      const tmpDir = await fs.promises.mkdtemp(
+        path.join(os.tmpdir(), 'polykey-'),
+      );
       try {
         pkClient = await PolykeyClient.createPolykeyClient({
           nodeId: clientOptions.nodeId,
@@ -54,45 +59,86 @@ class CommandEdit extends CommandPolykey {
           },
           logger: this.logger.getChild(PolykeyClient.name),
         });
-        const response = await binUtils.retryAuthentication(
-          (auth) =>
-            pkClient.rpcClient.methods.vaultsSecretsGet({
-              metadata: auth,
-              nameOrId: secretPath[0],
-              secretName: secretPath[1],
-            }),
+        const tmpFile = path.join(tmpDir, path.basename(secretPath[1]));
+        const secretExists = await binUtils.retryAuthentication(
+          async (auth) => {
+            let exists: boolean = true;
+            try {
+              const response =
+                await pkClient.rpcClient.methods.vaultsSecretsGet({
+                  metadata: auth,
+                  nameOrId: secretPath[0],
+                  secretName: secretPath[1],
+                });
+              await this.fs.promises.writeFile(tmpFile, response.secretContent);
+            } catch (e) {
+              const [cause, _] = binUtils.remoteErrorCause(e);
+              if (cause instanceof vaultsErrors.ErrorSecretsSecretUndefined) {
+                exists = false;
+              } else {
+                throw e;
+              }
+            }
+            return exists;
+          },
           meta,
         );
-        const secretContent = response.secretContent;
-        const tmpDir = await fs.promises.mkdtemp(
-          path.join(os.tmpdir(), 'polykey-'),
-        );
-        const tmpFile = path.join(tmpDir, 'pksecret');
-        await this.fs.promises.writeFile(tmpFile, secretContent);
-        execSync(`$EDITOR \"${tmpFile}\"`, { stdio: 'inherit' });
-        let content: Buffer;
+        // If the editor exited with a code other than zero, then execSync
+        // will throw an error. So, in the case of saving the file but the
+        // editor crashing, the program won't save the updated secret.
+        execSync(`${process.env.EDITOR} \"${tmpFile}\"`, { stdio: 'inherit' });
+        let content: string;
         try {
-          content = await this.fs.promises.readFile(tmpFile);
+          content = (await this.fs.promises.readFile(tmpFile)).toString(
+            'binary',
+          );
         } catch (e) {
-          throw new errors.ErrorPolykeyCLIFileRead(e.message, {
-            data: {
-              errno: e.errno,
-              syscall: e.syscall,
-              code: e.code,
-              path: e.path,
-            },
-            cause: e,
-          });
+          if (e.code === 'ENOENT') {
+            // If the secret exists but the file doesn't, then something went
+            // wrong, and the file cannot be read anymore. This is bad.
+            if (secretExists) {
+              throw new errors.ErrorPolykeyCLIFileRead(e.message, {
+                data: {
+                  errno: e.errno,
+                  syscall: e.syscall,
+                  code: e.code,
+                  path: e.path,
+                },
+                cause: e,
+              });
+              // If the secret didn't exist before and we can't read the file,
+              // then the secret was never actually created or saved. The user
+              // doesn't want to make the secret anymore, so abort mision!
+            } else {
+              return;
+            }
+          }
+          throw e;
         }
-        await pkClient.rpcClient.methods.vaultsSecretsEdit({
-          nameOrId: secretPath[0],
-          secretName: secretPath[1],
-          secretContent: content.toString('binary'),
-        });
-        await this.fs.promises.rm(tmpDir, { recursive: true, force: true });
+        await binUtils.retryAuthentication(async (auth) => {
+          // This point will never be reached if the temp file doesn't exist.
+          // As such, if the secret didn't exist before, then we want to make it.
+          // Otherwise, if the secret existed before, then we want to edit it.
+          if (secretExists) {
+            await pkClient.rpcClient.methods.vaultsSecretsEdit({
+              metadata: auth,
+              nameOrId: secretPath[0],
+              secretName: secretPath[1],
+              secretContent: content,
+            });
+          } else {
+            await pkClient.rpcClient.methods.vaultsSecretsNew({
+              metadata: auth,
+              nameOrId: secretPath[0],
+              secretName: secretPath[1],
+              secretContent: content,
+            });
+          }
+        }, meta);
         // Windows
         // TODO: complete windows impl
       } finally {
+        await this.fs.promises.rm(tmpDir, { recursive: true, force: true });
         if (pkClient! != null) await pkClient.stop();
       }
     });

tests/secrets/edit.test.ts

@@ -0,0 +1,178 @@
+import type { VaultName } from 'polykey/dist/vaults/types';
+import path from 'path';
+import fs from 'fs';
+import Logger, { LogLevel, StreamHandler } from '@matrixai/logger';
+import PolykeyAgent from 'polykey/dist/PolykeyAgent';
+import { vaultOps } from 'polykey/dist/vaults';
+import * as keysUtils from 'polykey/dist/keys/utils';
+import * as testUtils from '../utils';
+
+describe('commandEditSecret', () => {
+  const password = 'password';
+  const logger = new Logger('CLI Test', LogLevel.WARN, [new StreamHandler()]);
+  const editedContent = 'edited secret contents';
+  let dataDir: string;
+  let editorEdit: string;
+  let editorExit: string;
+  let editorFail: string;
+  let editorView: string;
+  let polykeyAgent: PolykeyAgent;
+  let command: Array<string>;
+
+  beforeEach(async () => {
+    dataDir = await fs.promises.mkdtemp(
+      path.join(globalThis.tmpDir, 'polykey-test-'),
+    );
+    editorEdit = path.join(dataDir, 'editorEdit.sh');
+    editorExit = path.join(dataDir, 'editorExit.sh');
+    editorFail = path.join(dataDir, 'editorFail.sh');
+    editorView = path.join(dataDir, 'editorView.sh');
+    await fs.promises.writeFile(editorExit, `#!/usr/bin/env bash\nexit`);
+    await fs.promises.chmod(editorExit, '755');
+    await fs.promises.writeFile(
+      editorView,
+      `#!/usr/bin/env bash\ncp $1 ${dataDir}/secret; echo "${editedContent}" > $1`,
+    );
+    await fs.promises.chmod(editorView, '755');
+    await fs.promises.writeFile(
+      editorEdit,
+      `#!/usr/bin/env bash\necho "${editedContent}" > $1`,
+    );
+    await fs.promises.chmod(editorEdit, '755');
+    await fs.promises.writeFile(
+      editorFail,
+      `#!/usr/bin/env bash\necho "${editedContent}" > $1; exit 1`,
+    );
+    await fs.promises.chmod(editorFail, '755');
+    polykeyAgent = await PolykeyAgent.createPolykeyAgent({
+      password,
+      options: {
+        nodePath: dataDir,
+        agentServiceHost: '127.0.0.1',
+        clientServiceHost: '127.0.0.1',
+        keys: {
+          passwordOpsLimit: keysUtils.passwordOpsLimits.min,
+          passwordMemLimit: keysUtils.passwordMemLimits.min,
+          strictMemoryLock: false,
+        },
+      },
+      logger: logger,
+    });
+  });
+  afterEach(async () => {
+    await polykeyAgent.stop();
+    await fs.promises.rm(dataDir, {
+      force: true,
+      recursive: true,
+    });
+  });
+
+  test('should edit secret', async () => {
+    const vaultName = 'Vault10' as VaultName;
+    const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
+    const secretName = 'secret';
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      await vaultOps.addSecret(vault, secretName, 'original secret');
+    });
+
+    command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
+
+    const result = await testUtils.pkStdio([...command], {
+      env: { PK_PASSWORD: password, EDITOR: editorEdit },
+      cwd: dataDir,
+    });
+    expect(result.exitCode).toBe(0);
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const contents = await vaultOps.getSecret(vault, secretName);
+      expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
+    });
+  });
+
+  test('should create secret if it does not exist', async () => {
+    const vaultName = 'Vault10' as VaultName;
+    const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
+    const secretName = 'secret';
+
+    command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
+
+    const result = await testUtils.pkStdio([...command], {
+      env: { PK_PASSWORD: password, EDITOR: editorEdit },
+      cwd: dataDir,
+    });
+    expect(result.exitCode).toBe(0);
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const contents = await vaultOps.getSecret(vault, secretName);
+      expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
+    });
+  });
+
+  test('should not create secret if editor crashes', async () => {
+    const vaultName = 'Vault10' as VaultName;
+    const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
+    const secretName = 'secret';
+
+    command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
+
+    const result = await testUtils.pkStdio([...command], {
+      env: { PK_PASSWORD: password, EDITOR: editorFail },
+      cwd: dataDir,
+    });
+    expect(result.exitCode).not.toBe(0);
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const list = await vaultOps.listSecrets(vault);
+      expect(list.sort()).toStrictEqual([]);
+    });
+  });
+
+  test('should not create secret if editor does not write to file', async () => {
+    const vaultName = 'Vault10' as VaultName;
+    const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
+    const secretName = 'secret';
+
+    command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
+
+    const result = await testUtils.pkStdio([...command], {
+      env: { PK_PASSWORD: password, EDITOR: editorExit },
+      cwd: dataDir,
+    });
+    expect(result.exitCode).toBe(0);
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const list = await vaultOps.listSecrets(vault);
+      expect(list.sort()).toStrictEqual([]);
+    });
+  });
+
+  test('file contents should be fetched correctly', async () => {
+    const vaultName = 'Vault10' as VaultName;
+    const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
+    const secretName = 'secret';
+    const secretContent = 'original secret';
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      await vaultOps.addSecret(vault, secretName, secretContent);
+    });
+
+    command = ['secrets', 'edit', '-np', dataDir, `${vaultName}:${secretName}`];
+
+    const result = await testUtils.pkStdio([...command], {
+      env: { PK_PASSWORD: password, EDITOR: editorView },
+      cwd: dataDir,
+    });
+    expect(result.exitCode).toBe(0);
+
+    const fetchedSecret = await fs.promises.readFile(
+      path.join(dataDir, 'secret'),
+    );
+    expect(fetchedSecret.toString()).toEqual(secretContent);
+
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const contents = await vaultOps.getSecret(vault, secretName);
+      expect(contents.toString()).toStrictEqual(`${editedContent}\n`);
+    });
+  });
+});

tests/secrets/get.test.ts

@@ -52,9 +52,7 @@ describe('commandGetSecret', () => {
     command = ['secrets', 'get', '-np', dataDir, `${vaultName}:MySecret`];
 
     const result = await testUtils.pkStdio([...command], {
-      env: {
-        PK_PASSWORD: password,
-      },
+      env: { PK_PASSWORD: password },
       cwd: dataDir,
     });
     expect(result.stdout).toBe('this is the secret');

tests/secrets/list.test.ts

@@ -84,7 +84,7 @@ describe('commandListSecrets', () => {
   test(
     'should fail when path is not a directory',
     async () => {
-      const vaultName = 'Vault5' as VaultName;
+      const vaultName = 'Vault4' as VaultName;
       const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
 
       await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
@@ -124,7 +124,7 @@ describe('commandListSecrets', () => {
   test(
     'should list secrets within directories',
     async () => {
-      const vaultName = 'Vault6' as VaultName;
+      const vaultName = 'Vault4' as VaultName;
       const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
 
       await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {