Merge pull request #417 from MatrixAI/feature-make-rename-accept-vault-path-for-both-params

Allow fully qualified paths for new secret name in secrets rename

Author: aryanjassal

.gitignore

@@ -130,3 +130,6 @@ dist
 # editor
 .vscode/
 .idea/
+
+# prettier for editor formatting
+.prettierrc

src/secrets/CommandRename.ts

@@ -16,15 +16,30 @@ class CommandRename extends CommandPolykey {
       'Path to where the secret to be renamed, specified as <vaultName>:<directoryPath>',
       binParsers.parseSecretPath,
     );
-    this.argument('<newSecretName>', 'New name of the secret');
+    this.argument(
+      '<newSecretName>',
+      'Fully-qualified new name for the secret, specified as <vaultName>:<secretPath>',
+      binParsers.parseSecretPath,
+    );
     this.addOption(binOptions.nodeId);
     this.addOption(binOptions.clientHost);
     this.addOption(binOptions.clientPort);
-    this.action(async (secretPath, newSecretName, options) => {
+    this.action(async (secretPath, newSecretPath, options) => {
+      // Rename operation cannot work across vaults, only within a vault
+      if (secretPath[0] !== newSecretPath[0]) {
+        throw new errors.ErrorPolykeyCLIRenameSecret(
+          'Cannot rename file into another vault',
+        );
+      }
       // Ensure that a valid secret path is provided
       if (secretPath[1] == null) {
         throw new errors.ErrorPolykeyCLIRenameSecret(
-          'EPERM: Cannot rename vault root',
+          'Cannot rename vault root',
+        );
+      }
+      if (newSecretPath[1] == null) {
+        throw new errors.ErrorPolykeyCLIRenameSecret(
+          'Cannot rename to vault root',
         );
       }
       const { default: PolykeyClient } = await import(
@@ -63,7 +78,7 @@ class CommandRename extends CommandPolykey {
               metadata: auth,
               nameOrId: secretPath[0],
               secretName: secretPath[1],
-              newSecretName: newSecretName,
+              newSecretName: newSecretPath[1],
             }),
           meta,
         );

tests/secrets/rename.test.ts

@@ -12,7 +12,6 @@ describe('commandRenameSecret', () => {
   const logger = new Logger('CLI Test', LogLevel.WARN, [new StreamHandler()]);
   let dataDir: string;
   let polykeyAgent: PolykeyAgent;
-  let command: Array<string>;
 
   beforeEach(async () => {
     dataDir = await fs.promises.mkdtemp(
@@ -33,6 +32,7 @@ describe('commandRenameSecret', () => {
       logger: logger,
     });
   });
+
   afterEach(async () => {
     await polykeyAgent.stop();
     await fs.promises.rm(dataDir, {
@@ -44,39 +44,86 @@ describe('commandRenameSecret', () => {
   test('should rename secrets', async () => {
     const vaultName = 'vault' as VaultName;
     const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
-    const secretName = 'secret';
-    const newSecretName = 'secret-renamed';
-    const secretContent = 'this is the secret';
+    const oldSecretName = 'secretOld';
+    const newSecretName = 'secretNew';
+    const secretContent = 'this is the secret for simple rename';
     await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
-      await vaultOps.addSecret(vault, secretName, secretContent);
+      await vaultOps.addSecret(vault, oldSecretName, secretContent);
     });
-    command = [
+
+    // Should fail if only new name is provided
+    const command1 = [
       'secrets',
       'rename',
       '-np',
       dataDir,
-      `${vaultName}:${secretName}`,
+      `${vaultName}:${oldSecretName}`,
       newSecretName,
     ];
-    const result = await testUtils.pkStdio(command, {
+    const result1 = await testUtils.pkStdio(command1, {
       env: { PK_PASSWORD: password },
       cwd: dataDir,
     });
-    expect(result.exitCode).toBe(0);
+    expect(result1.exitCode).toBe(1);
+    await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
+      const list = await vaultOps.listSecrets(vault);
+      expect(list.sort()).toStrictEqual([oldSecretName]);
+    });
+
+    // Should pass if fully-qualified path is provided
+    const command2 = [
+      'secrets',
+      'rename',
+      '-np',
+      dataDir,
+      `${vaultName}:${oldSecretName}`,
+      `${vaultName}:${newSecretName}`,
+    ];
+    const result2 = await testUtils.pkStdio(command2, {
+      env: { PK_PASSWORD: password },
+      cwd: dataDir,
+    });
+    expect(result2.exitCode).toBe(0);
     await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => {
       const list = await vaultOps.listSecrets(vault);
       expect(list.sort()).toStrictEqual([newSecretName]);
     });
   });
-  test('should not rename vault root', async () => {
-    const vaultName = 'vault' as VaultName;
-    await polykeyAgent.vaultManager.createVault(vaultName);
-    command = ['secrets', 'rename', '-np', dataDir, vaultName, 'rename'];
+
+  test('should fail renaming across vaults', async () => {
+    const vaultName1 = 'vault1' as VaultName;
+    const vaultName2 = 'vault2' as VaultName;
+    const vaultId1 = await polykeyAgent.vaultManager.createVault(vaultName1);
+    const vaultId2 = await polykeyAgent.vaultManager.createVault(vaultName2);
+    const oldSecretName = 'secretOld';
+    const newSecretName = 'secretNew';
+    const secretContent = 'this is the secret for simple rename';
+    await polykeyAgent.vaultManager.withVaults([vaultId1], async (vault) => {
+      await vaultOps.addSecret(vault, oldSecretName, secretContent);
+    });
+    const command = [
+      'secrets',
+      'rename',
+      '-np',
+      dataDir,
+      `${vaultName1}:${oldSecretName}`,
+      `${vaultName2}:${newSecretName}`,
+    ];
     const result = await testUtils.pkStdio(command, {
       env: { PK_PASSWORD: password },
       cwd: dataDir,
     });
-    expect(result.exitCode).not.toBe(0);
-    expect(result.stderr).toInclude('EPERM');
+    expect(result.exitCode).toBe(1);
+    await polykeyAgent.vaultManager.withVaults(
+      [vaultId1, vaultId2],
+      async (vault1, vault2) => {
+        // The secret wasn't changed in the original place
+        const list1 = await vaultOps.listSecrets(vault1);
+        expect(list1.sort()).toStrictEqual([oldSecretName]);
+        // The target vault is also unchanged
+        const list2 = await vaultOps.listSecrets(vault2);
+        expect(list2.sort()).toStrictEqual([]);
+      },
+    );
   });
 });