feat: updated tests for every secret command to allow undefined secret path

Author: aryanjassal

src/errors.ts

@@ -157,6 +157,21 @@ class ErrorPolykeyCLIMakeDirectory<T> extends ErrorPolykeyCLI<T> {
   exitCode = 1;
 }
 
+class ErrorPolykeyCLIRenameSecret<T> extends ErrorPolykeyCLI<T> {
+  static description = 'Failed to rename one or more secrets';
+  exitCode = 1;
+}
+
+class ErrorPolykeyCLIRemoveSecret<T> extends ErrorPolykeyCLI<T> {
+  static description = 'Failed to remove one or more secrets';
+  exitCode = 1;
+}
+
+class ErrorPolykeyCLICatSecret<T> extends ErrorPolykeyCLI<T> {
+  static description = 'Failed to concatenate one or more secrets';
+  exitCode = 1;
+}
+
 export {
   ErrorPolykeyCLI,
   ErrorPolykeyCLIUncaughtException,
@@ -178,4 +193,7 @@ export {
   ErrorPolykeyCLIInvalidEnvName,
   ErrorPolykeyCLIDuplicateEnvName,
   ErrorPolykeyCLIMakeDirectory,
+  ErrorPolykeyCLIRenameSecret,
+  ErrorPolykeyCLIRemoveSecret,
+  ErrorPolykeyCLICatSecret,
 };

src/secrets/CommandCat.ts

@@ -4,6 +4,7 @@ import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
 import * as binParsers from '../utils/parsers';
 import * as binProcessors from '../utils/processors';
+import * as errors from '../errors';
 
 class CommandGet extends CommandPolykey {
   constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
@@ -21,7 +22,7 @@ class CommandGet extends CommandPolykey {
     this.addOption(binOptions.clientPort);
     this.action(async (secretPaths, options) => {
       secretPaths = secretPaths.map((path: string) =>
-        binParsers.parseSecretPathValue(path),
+        binParsers.parseSecretPath(path),
       );
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
@@ -77,29 +78,37 @@ class CommandGet extends CommandPolykey {
           });
           return;
         }
-        await binUtils.retryAuthentication(async (auth) => {
+        const response = await binUtils.retryAuthentication(async (auth) => {
           const response = await pkClient.rpcClient.methods.vaultsSecretsGet();
-          await (async () => {
-            const writer = response.writable.getWriter();
-            let first = true;
-            for (const [vaultName, secretPath] of secretPaths) {
-              await writer.write({
-                nameOrId: vaultName,
-                secretName: secretPath ?? '/',
-                metadata: first
-                  ? { ...auth, options: { continueOnError: true } }
-                  : undefined,
-              });
-              first = false;
-            }
-            await writer.close();
-          })();
-          for await (const chunk of response.readable) {
-            if (chunk.error) process.stderr.write(chunk.error);
-            else process.stdout.write(chunk.secretContent);
+          const writer = response.writable.getWriter();
+          let first = true;
+          for (const [vaultName, secretPath] of secretPaths) {
+            await writer.write({
+              nameOrId: vaultName,
+              secretName: secretPath ?? '/',
+              metadata: first
+                ? { ...auth, options: { continueOnError: true } }
+                : undefined,
+            });
+            first = false;
           }
-          process.stderr.write("\n");
+          await writer.close();
+          return response;
         }, meta);
+        let hasErrored = false;
+        for await (const chunk of response.readable) {
+          if (chunk.error) {
+            hasErrored = true;
+            process.stderr.write(chunk.error);
+          } else {
+            process.stdout.write(chunk.secretContent);
+          }
+        }
+        if (hasErrored) {
+          throw new errors.ErrorPolykeyCLICatSecret(
+            'Failed to concatenate one or more secrets',
+          );
+        }
       } finally {
         if (pkClient! != null) await pkClient.stop();
       }

src/secrets/CommandCreate.ts

@@ -73,7 +73,7 @@ class CommandCreate extends CommandPolykey {
             pkClient.rpcClient.methods.vaultsSecretsNew({
               metadata: auth,
               nameOrId: secretPath[0],
-              secretName: secretPath[1],
+              secretName: secretPath[1] ?? '/',
               secretContent: content.toString('binary'),
             }),
           meta,

src/secrets/CommandRemove.ts

@@ -4,6 +4,7 @@ import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
 import * as binParsers from '../utils/parsers';
 import * as binProcessors from '../utils/processors';
+import * as errors from '../errors';
 
 class CommandRemove extends CommandPolykey {
   constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
@@ -20,9 +21,17 @@ class CommandRemove extends CommandPolykey {
     this.addOption(binOptions.clientPort);
     this.addOption(binOptions.recursive);
     this.action(async (secretPaths, options) => {
-      secretPaths = secretPaths.map((path: string) =>
-        binParsers.parseSecretPath(path),
-      );
+      for (let i = 0; i < secretPaths.length; i++) {
+        const value: string = secretPaths[i];
+        const parsedValue = binParsers.parseSecretPath(value);
+        // The vault root cannot be deleted
+        if (parsedValue[1] == null) {
+          throw new errors.ErrorPolykeyCLIRemoveSecret(
+            'EPERM: Cannot remove vault root',
+          );
+        }
+        secretPaths[i] = parsedValue;
+      }
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
       );

src/secrets/CommandRename.ts

@@ -4,6 +4,7 @@ import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
 import * as binParsers from '../utils/parsers';
 import * as binProcessors from '../utils/processors';
+import * as errors from '../errors';
 
 class CommandRename extends CommandPolykey {
   constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
@@ -13,13 +14,19 @@ class CommandRename extends CommandPolykey {
     this.argument(
       '<secretPath>',
       'Path to where the secret to be renamed, specified as <vaultName>:<directoryPath>',
-      binParsers.parseSecretPathValue,
+      binParsers.parseSecretPath,
     );
     this.argument('<newSecretName>', 'New name of the secret');
     this.addOption(binOptions.nodeId);
     this.addOption(binOptions.clientHost);
     this.addOption(binOptions.clientPort);
     this.action(async (secretPath, newSecretName, options) => {
+      // Ensure that a valid secret path is provided
+      if (secretPath[1] == null) {
+        throw new errors.ErrorPolykeyCLIRenameSecret(
+          'EPERM: Cannot rename vault root',
+        );
+      }
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
       );

src/secrets/CommandStat.ts

@@ -55,7 +55,7 @@ class CommandStat extends CommandPolykey {
             pkClient.rpcClient.methods.vaultsSecretsStat({
               metadata: auth,
               nameOrId: secretPath[0],
-              secretName: secretPath[1],
+              secretName: secretPath[1] ?? '/',
             }),
           meta,
         );

src/utils/parsers.ts

@@ -10,7 +10,6 @@ import * as nodesUtils from 'polykey/dist/nodes/utils';
 
 const vaultNameRegex = /^(?!.*[:])[ -~\t\n]*$/s;
 const secretPathRegex = /^(?!.*[=])[ -~\t\n]*$/s;
-const vaultNameSecretPathRegex = /^([\w\-\.]+)(?::([^\0\\=]+))?$/;
 const secretPathValueRegex = /^([a-zA-Z_][\w]+)?$/;
 const environmentVariableRegex = /^([a-zA-Z_]+[a-zA-Z0-9_]*)?$/;
 
@@ -76,52 +75,54 @@ function parseVaultName(vaultName: string): string {
   return vaultName;
 }
 
-// E.g. If 'vault1:a/b/c', ['vault1', 'a/b/c'] is returned
-//      If 'vault1', ['vault1, undefined] is returned
+// E.g. If 'vault1:a/b/c', ['vault1', 'a/b/c', undefined] is returned
+//      If 'vault1', ['vault1', undefined, undefined] is returned
+//      If 'vault1:abc=xyz', ['vault1', 'abc', 'xyz'] is returned
 //      If 'vault1:', an error is thrown
 //      If 'a/b/c', an error is thrown
 // Splits out everything after an `=` separator
-function parseSecretPath(secretPath: string): [string, string?, string?] {
+function parseSecretPath(inputPath: string): [string, string?, string?] {
   // The colon character `:` is prohibited in vaultName, so it's first occurence
   // means that this is the delimiter between vaultName and secretPath.
-  const colonIndex = secretPath.indexOf(':');
+  const colonIndex = inputPath.indexOf(':');
   // If no colon exists, treat entire string as vault name
   if (colonIndex === -1) {
-    return [parseVaultName(secretPath), undefined, undefined];
+    return [parseVaultName(inputPath), undefined, undefined];
   }
-  // Calculate contents before the `=` separator
-  const vaultNamePart = secretPath.substring(0, colonIndex);
-  const secretPathPart = secretPath.substring(colonIndex + 1);
-  // Calculate contents after the `=` separator
+  // Calculate vaultName and secretPath
+  const vaultNamePart = inputPath.substring(0, colonIndex);
+  const secretPathPart = inputPath.substring(colonIndex + 1);
+  // Calculate contents after the `=` separator (value)
   const equalIndex = secretPathPart.indexOf('=');
-  const splitSecretPath =
+  // If `=` isn't found, then the entire secret path section is the actual path.
+  // Otherwise, split the path section by the index of `=`. First half is the
+  // actual secret part, and the second half is the value.
+  const secretPath =
     equalIndex === -1
       ? secretPathPart
       : secretPathPart.substring(0, equalIndex);
   const value =
-    equalIndex === -1 ? undefined : secretPathPart.substring(equalIndex + 1);
-  if (splitSecretPath != null && !secretPathRegex.test(splitSecretPath)) {
+    equalIndex !== -1 ? secretPathPart.substring(equalIndex + 1) : undefined;
+  // If secretPath exists but it doesn't pass the regex test, then the path is
+  // malformed.
+  if (secretPath != null && !secretPathRegex.test(secretPath)) {
     throw new commander.InvalidArgumentError(
-      `${secretPath} is not of the format <vaultName>[:<secretPath>][=<value>]`,
+      `${inputPath} is not of the format <vaultName>[:<secretPath>][=<value>]`,
     );
   }
-  const parsedVaultName = parseVaultName(vaultNamePart);
-  const parsedSecretPath = splitSecretPath.match(secretPathRegex)?.[0];
-  return [parsedVaultName, parsedSecretPath, value];
+  // We have already tested if the secretPath is valid, and we can return it
+  // as-is.
+  const vaultName = parseVaultName(vaultNamePart);
+  return [vaultName, secretPath, value];
 }
 
-function parseSecretPathValue(secretPath: string): [string, string, string?] {
+function parseSecretPathValue(secretPath: string): [string, string?, string?] {
   const [vaultName, directoryPath, value] = parseSecretPath(secretPath);
   if (value != null && !secretPathValueRegex.test(value)) {
     throw new commander.InvalidArgumentError(
       `${value} is not a valid value name`,
     );
   }
-  if (directoryPath == null) {
-    throw new commander.InvalidArgumentError(
-      `${secretPath} is not of the format <vaultName>:<directoryPath>[=<value>]`,
-    );
-  }
   return [vaultName, directoryPath, value];
 }
 

src/vaults/CommandList.ts

@@ -8,8 +8,9 @@ import * as binProcessors from '../utils/processors';
 class CommandList extends CommandPolykey {
   constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
     super(...args);
-    this.name('list');
-    this.description('List all Available Vaults');
+    this.name('ls');
+    this.alias('list');
+    this.description('List all available Vaults');
     this.addOption(binOptions.nodeId);
     this.addOption(binOptions.clientHost);
     this.addOption(binOptions.clientPort);

tests/secrets/cat.test.ts

@@ -40,7 +40,7 @@ describe('commandCatSecret', () => {
     });
   });
   test('should retrieve a secret', async () => {
-    const vaultName = 'Vault3' as VaultName;
+    const vaultName = 'vault' as VaultName;
     const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
     const secretName = 'secret-name';
     const secretContent = 'this is the contents of the secret';
@@ -62,7 +62,7 @@ describe('commandCatSecret', () => {
     expect(result.stdout).toBe(secretContent);
   });
   test('should fail when reading root without secret path', async () => {
-    const vaultName = 'Vault3' as VaultName;
+    const vaultName = 'vault' as VaultName;
     const vaultId = await polykeyAgent.vaultManager.createVault(vaultName);
     const secretName = 'secret-name';
     const secretContent = 'this is the contents of the secret';
@@ -75,7 +75,7 @@ describe('commandCatSecret', () => {
       cwd: dataDir,
     });
     expect(result.exitCode).not.toBe(0);
-    expect(result.stderr).toBeDefined();
+    expect(result.stderr).toInclude('ErrorSecretsIsDirectory');
   });
   test('should concatenate multiple secrets', async () => {
     const vaultName = 'Vault3' as VaultName;
@@ -148,14 +148,14 @@ describe('commandCatSecret', () => {
       '-np',
       dataDir,
       `${vaultName}:${secretName1}`,
-      `${vaultName}:doesnt-exist-file`,
+      `${vaultName}:doesnt-exist`,
       `${vaultName}:${secretName2}`,
     ];
     const result = await testUtils.pkStdio(command, {
       env: { PK_PASSWORD: password },
       cwd: dataDir,
     });
-    expect(result.exitCode).toBe(0);
+    expect(result.exitCode).toBe(1);
     expect(result.stdout).toBe(`${secretName1}${secretName2}`);
     expect(result.stderr).not.toBe('');
   });