diff --git a/src/errors.ts b/src/errors.ts index 40314093..a524a9eb 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -84,8 +84,8 @@ class ErrorPolykeyCLIUnexpectedError extends ErrorPolykeyCLI { exitCode = sysexits.SOFTWARE; } -class ErrorPolykeyCLISubprocessFailure extends ErrorPolykeyCLI { - static description = 'A subprocess failed to exit gracefully'; +class ErrorPolykeyCLIChildProcessFailure extends ErrorPolykeyCLI { + static description = 'A child process failed to exit gracefully'; exitCode = sysexits.UNKNOWN; } @@ -196,7 +196,7 @@ export { ErrorPolykeyCLIUncaughtException, ErrorPolykeyCLIUnhandledRejection, ErrorPolykeyCLIUnexpectedError, - ErrorPolykeyCLISubprocessFailure, + ErrorPolykeyCLIChildProcessFailure, ErrorPolykeyCLIAsynchronousDeadlock, ErrorPolykeyCLINodePath, ErrorPolykeyCLIClientOptions, diff --git a/src/secrets/CommandEdit.ts b/src/secrets/CommandEdit.ts index 672b9ced..7ea722f9 100644 --- a/src/secrets/CommandEdit.ts +++ b/src/secrets/CommandEdit.ts @@ -120,7 +120,7 @@ class CommandEdit extends CommandPolykey { }; const onError = (e: Error) => { cleanup(); - const error = new errors.ErrorPolykeyCLISubprocessFailure( + const error = new errors.ErrorPolykeyCLIChildProcessFailure( `Failed to run command '${process.env.EDITOR}'`, { cause: e }, ); @@ -129,7 +129,7 @@ class CommandEdit extends CommandPolykey { const onClose = (code: number | null) => { cleanup(); if (code !== 0) { - const error = new errors.ErrorPolykeyCLISubprocessFailure( + const error = new errors.ErrorPolykeyCLIChildProcessFailure( `Editor exited with code ${code}`, ); reject(error); diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index 453c0561..755c0b4a 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -2,6 +2,7 @@ import type PolykeyClient from 'polykey/dist/PolykeyClient'; import type { ParsedSecretPathValue } from '../types'; import path from 'path'; import os from 'os'; +import commander from 'commander'; import * as utils from 'polykey/dist/utils'; import CommandPolykey from '../CommandPolykey'; import * as binProcessors from '../utils/processors'; @@ -26,297 +27,324 @@ class CommandEnv extends CommandPolykey { this.addOption(binOptions.preserveNewline); this.argument( '', - 'command and arguments formatted as -- [ cmd [cmdArgs...]]', - binParsers.parseEnvArgs, + 'command and arguments formatted as [-- cmd [cmdArgs...]]', ); - this.action( - async (args: [Array, Array], options) => { - const { default: PolykeyClient } = await import( - 'polykey/dist/PolykeyClient' - ); - const { - envInvalid, - envDuplicate, - envFormat, - }: { - envInvalid: 'error' | 'warn' | 'ignore'; - envDuplicate: 'keep' | 'overwrite' | 'warn' | 'error'; - envFormat: 'auto' | 'unix' | 'cmd' | 'powershell' | 'json'; - } = options; - // Populate a set with all the paths we want to preserve newlines for - const preservedSecrets = new Set(); - for (const [vaultName, secretPath] of options.preserveNewline) { - // The vault name is guaranteed to have a value. - // If a secret path is undefined, then the newline preservation was - // targeting the secrets of the entire vault. Otherwise, the target - // was a single secret. - if (secretPath == null) preservedSecrets.add(vaultName); - else preservedSecrets.add(`${vaultName}:${secretPath}`); + this.action(async (args, options) => { + const { default: PolykeyClient } = await import( + 'polykey/dist/PolykeyClient' + ); + const { + envInvalid, + envDuplicate, + envFormat, + }: { + envInvalid: 'error' | 'warn' | 'ignore'; + envDuplicate: 'keep' | 'overwrite' | 'warn' | 'error'; + envFormat: 'auto' | 'unix' | 'cmd' | 'powershell' | 'json'; + } = options; + // Populate a set with all the paths we want to preserve newlines for + const preservedSecrets = new Set(); + for (const [vaultName, secretPath] of options.preserveNewline) { + // The vault name is guaranteed to have a value. + // If a secret path is undefined, then the newline preservation was + // targeting the secrets of the entire vault. Otherwise, the target + // was a single secret. + if (secretPath == null) preservedSecrets.add(vaultName); + else preservedSecrets.add(`${vaultName}:${secretPath}`); + } + // There are a few stages here + // 1. parse the desired secrets + // 2. obtain the desired secrets + // 3. switching behaviour here based on parameters + // a. exec the command with the provided env variables from the secrets + // b. output the env variables in the desired format + + // Emulate commander's parsing to allow for parsing -- + const fullArgs = process.argv; + const fullSeparatorIndex = fullArgs.indexOf('--'); + const envVariables: Array = []; + let cmd: string | undefined; + let argv: Array = []; + if (fullSeparatorIndex === -1) { + // If the separator exists, then treat all args as environment paths + for (const arg of args) { + envVariables.push(binParsers.parseSecretPathEnv(arg)); } - // There are a few stages here - // 1. parse the desired secrets - // 2. obtain the desired secrets - // 3. switching behaviour here based on parameters - // a. exec the command with the provided env variables from the secrets - // b. output the env variables in the desired format - const [envVariables, [cmd, ...argv]] = args; - const clientOptions = await binProcessors.processClientOptions( - options.nodePath, - options.nodeId, - options.clientHost, - options.clientPort, - this.fs, - this.logger.getChild(binProcessors.processClientOptions.name), - ); - const meta = await binProcessors.processAuthentication( - options.passwordFile, - this.fs, + } else { + // Otherwise, separate command from environment paths + const afterSeparatorCount = fullArgs.length - fullSeparatorIndex - 1; + const cmdIndex = args.length - afterSeparatorCount; + const rawEnvVariables = args.slice(0, -afterSeparatorCount); + // Parse environment variables correctly + for (const arg of rawEnvVariables) { + envVariables.push(binParsers.parseSecretPathEnv(arg)); + } + cmd = args[cmdIndex]; + argv = args.slice(cmdIndex + 1); + } + if (envVariables.length === 0) { + this.addHelpText('before', 'You must provide at least 1 secret path'); + this.outputHelp(); + throw new commander.InvalidArgumentError( + 'You must provide at least 1 secret path', ); + } + const clientOptions = await binProcessors.processClientOptions( + options.nodePath, + options.nodeId, + options.clientHost, + options.clientPort, + this.fs, + this.logger.getChild(binProcessors.processClientOptions.name), + ); + const meta = await binProcessors.processAuthentication( + options.passwordFile, + this.fs, + ); - let pkClient: PolykeyClient; - this.exitHandlers.handlers.push(async () => { - if (pkClient != null) await pkClient.stop(); + let pkClient: PolykeyClient; + this.exitHandlers.handlers.push(async () => { + if (pkClient != null) await pkClient.stop(); + }); + try { + pkClient = await PolykeyClient.createPolykeyClient({ + nodeId: clientOptions.nodeId, + host: clientOptions.clientHost, + port: clientOptions.clientPort, + options: { + nodePath: options.nodePath, + }, + logger: this.logger.getChild(PolykeyClient.name), }); - try { - pkClient = await PolykeyClient.createPolykeyClient({ - nodeId: clientOptions.nodeId, - host: clientOptions.clientHost, - port: clientOptions.clientPort, - options: { - nodePath: options.nodePath, - }, - logger: this.logger.getChild(PolykeyClient.name), - }); - // Getting envs - const [envp] = await binUtils.retryAuthentication(async (auth) => { - const responseStream = - await pkClient.rpcClient.methods.vaultsSecretsEnv(); - // Writing desired secrets - const secretRenameMap = new Map(); - const writeP = (async () => { - const writer = responseStream.writable.getWriter(); - let first = true; - for (const envVariable of envVariables) { - const [nameOrId, secretName, secretNameNew] = envVariable; - secretRenameMap.set(secretName ?? '/', secretNameNew); - await writer.write({ - nameOrId: nameOrId, - secretName: secretName ?? '/', - metadata: first ? auth : undefined, - }); - first = false; - } - await writer.close(); - })(); + // Getting envs + const [envp] = await binUtils.retryAuthentication(async (auth) => { + const responseStream = + await pkClient.rpcClient.methods.vaultsSecretsEnv(); + // Writing desired secrets + const secretRenameMap = new Map(); + const writeP = (async () => { + const writer = responseStream.writable.getWriter(); + let first = true; + for (const envVariable of envVariables) { + const [nameOrId, secretName, secretNameNew] = envVariable; + secretRenameMap.set(secretName ?? '/', secretNameNew); + await writer.write({ + nameOrId: nameOrId, + secretName: secretName ?? '/', + metadata: first ? auth : undefined, + }); + first = false; + } + await writer.close(); + })(); - const envp: Record = {}; - const envpPath: Record< - string, - { - nameOrId: string; - secretName: string; - } - > = {}; - for await (const value of responseStream.readable) { - const { nameOrId, secretName, secretContent } = value; - let newName = secretRenameMap.get(secretName); - if (newName == null) { - const secretEnvName = path.basename(secretName); - // Validating name - if (!binUtils.validEnvRegex.test(secretEnvName)) { - switch (envInvalid) { - case 'error': - throw new binErrors.ErrorPolykeyCLIInvalidEnvName( - `The following env variable name (${secretEnvName}) is invalid`, - ); - case 'warn': - this.logger.warn( - `The following env variable name (${secretEnvName}) is invalid and was dropped`, - ); - // Fallthrough - case 'ignore': - continue; - default: - utils.never(); - } - } - newName = secretEnvName; - } - // Handling duplicate names - if (envp[newName] != null) { - switch (envDuplicate) { - // Continue without modifying + const envp: Record = {}; + const envpPath: Record< + string, + { + nameOrId: string; + secretName: string; + } + > = {}; + for await (const value of responseStream.readable) { + const { nameOrId, secretName, secretContent } = value; + let newName = secretRenameMap.get(secretName); + if (newName == null) { + const secretEnvName = path.basename(secretName); + // Validating name + if (!binUtils.validEnvRegex.test(secretEnvName)) { + switch (envInvalid) { case 'error': - throw new binErrors.ErrorPolykeyCLIDuplicateEnvName( - `The env variable (${newName}) is duplicate`, + throw new binErrors.ErrorPolykeyCLIInvalidEnvName( + `The following env variable name (${secretEnvName}) is invalid`, ); - // Fallthrough - case 'keep': - continue; - // Log a warning and overwrite case 'warn': this.logger.warn( - `The env variable (${newName}) is duplicate, overwriting`, + `The following env variable name (${secretEnvName}) is invalid and was dropped`, ); // Fallthrough - case 'overwrite': - break; + case 'ignore': + continue; default: utils.never(); } } - - // Find if we need to preserve the newline for this secret - let preserveNewline = false; - // If only the vault name is specified to be preserved, then - // preserve the newlines of all secrets inside the vault. - // Otherwise, if a full secret path has been specified, then - // preserve that secret path. - if ( - preservedSecrets.has(nameOrId) || - preservedSecrets.has(`${nameOrId}:${newName}`) - ) { - preserveNewline = true; + newName = secretEnvName; + } + // Handling duplicate names + if (envp[newName] != null) { + switch (envDuplicate) { + // Continue without modifying + case 'error': + throw new binErrors.ErrorPolykeyCLIDuplicateEnvName( + `The env variable (${newName}) is duplicate`, + ); + // Fallthrough + case 'keep': + continue; + // Log a warning and overwrite + case 'warn': + this.logger.warn( + `The env variable (${newName}) is duplicate, overwriting`, + ); + // Fallthrough + case 'overwrite': + break; + default: + utils.never(); } + } - // Trim the single trailing newline if it exists - if (!preserveNewline && secretContent.endsWith('\n')) { - envp[newName] = secretContent.slice(0, -1); - } else { - envp[newName] = secretContent; - } - envpPath[newName] = { - nameOrId, - secretName, - }; + // Find if we need to preserve the newline for this secret + let preserveNewline = false; + // If only the vault name is specified to be preserved, then + // preserve the newlines of all secrets inside the vault. + // Otherwise, if a full secret path has been specified, then + // preserve that secret path. + if ( + preservedSecrets.has(nameOrId) || + preservedSecrets.has(`${nameOrId}:${newName}`) + ) { + preserveNewline = true; } - await writeP; - return [envp, envpPath]; - }, meta); - // End connection early to avoid errors on server - await pkClient.stop(); - // Here we want to switch between the different usages - const platform = os.platform(); - if (cmd != null) { - // If a cmd is provided then we default to exec it - switch (platform) { - case 'linux': // Fallthrough - case 'darwin': - { - const { exec } = await import('@matrixai/exec'); - try { - exec.execvp(cmd, argv, envp); - } catch (e) { - if ('code' in e && e.code === 'GenericFailure') { - throw new binErrors.ErrorPolykeyCLISubprocessFailure( - `Command failed with error ${e}`, - { - cause: e, - data: { command: [cmd, ...argv] }, - }, - ); - } - throw e; + // Trim the single trailing newline if it exists + if (!preserveNewline && secretContent.endsWith('\n')) { + envp[newName] = secretContent.slice(0, -1); + } else { + envp[newName] = secretContent; + } + envpPath[newName] = { + nameOrId, + secretName, + }; + } + await writeP; + return [envp, envpPath]; + }, meta); + // End connection early to avoid errors on server + await pkClient.stop(); + + // Here we want to switch between the different usages + const platform = os.platform(); + if (cmd != null) { + // If a cmd is provided then we default to exec it + switch (platform) { + case 'linux': // Fallthrough + case 'darwin': + { + const { exec } = await import('@matrixai/exec'); + try { + exec.execvp(cmd, argv, envp); + } catch (e) { + if ('code' in e && e.code === 'GenericFailure') { + throw new binErrors.ErrorPolykeyCLIChildProcessFailure( + `Command failed with error ${e}`, + { + cause: e, + data: { command: [cmd, ...argv] }, + }, + ); } + throw e; } - break; - default: { - const { spawnSync } = await import('child_process'); - const result = spawnSync(cmd, argv, { - env: { - ...process.env, - ...envp, - }, - shell: false, - windowsHide: true, - stdio: 'inherit', - }); - process.exit(result.status ?? 255); } + break; + default: { + const { spawnSync } = await import('child_process'); + const result = spawnSync(cmd, argv, { + env: { + ...process.env, + ...envp, + }, + shell: false, + windowsHide: true, + stdio: 'inherit', + }); + process.exit(result.status ?? 255); } - } else { - // Otherwise we switch between output formats - // If set to `auto` then we need to infer the format - let format = envFormat; - if (envFormat === 'auto') { - format = - { - darwin: 'unix', - linux: 'unix', - win32: 'cmd', - }[platform] ?? 'unix'; - } - switch (format) { - case 'unix': - { - // Formatting as a .env file - let data = ''; - for (const [key, value] of Object.entries(envp)) { - data += `${key}='${value}'\n`; - } - process.stdout.write( - binUtils.outputFormatter({ - type: 'raw', - data, - }), - ); + } + } else { + // Otherwise we switch between output formats + // If set to `auto` then we need to infer the format + let format = envFormat; + if (envFormat === 'auto') { + format = + { + darwin: 'unix', + linux: 'unix', + win32: 'cmd', + }[platform] ?? 'unix'; + } + switch (format) { + case 'unix': + { + // Formatting as a .env file + let data = ''; + for (const [key, value] of Object.entries(envp)) { + data += `${key}='${value}'\n`; } - break; - case 'cmd': - { - // Formatting as a .bat file for windows cmd - let data = ''; - for (const [key, value] of Object.entries(envp)) { - data += `set "${key}=${value}"\n`; - } - process.stdout.write( - binUtils.outputFormatter({ - type: 'raw', - data, - }), - ); + process.stdout.write( + binUtils.outputFormatter({ + type: 'raw', + data, + }), + ); + } + break; + case 'cmd': + { + // Formatting as a .bat file for windows cmd + let data = ''; + for (const [key, value] of Object.entries(envp)) { + data += `set "${key}=${value}"\n`; } - break; - case 'powershell': - { - // Formatting as a .bat file for windows cmd - let data = ''; - for (const [key, value] of Object.entries(envp)) { - data += `\$env:${key} = '${value}'\n`; - } - process.stdout.write( - binUtils.outputFormatter({ - type: 'raw', - data, - }), - ); + process.stdout.write( + binUtils.outputFormatter({ + type: 'raw', + data, + }), + ); + } + break; + case 'powershell': + { + // Formatting as a .bat file for windows cmd + let data = ''; + for (const [key, value] of Object.entries(envp)) { + data += `\$env:${key} = '${value}'\n`; } - break; - case 'json': - { - const data = {}; - for (const [key, value] of Object.entries(envp)) { - data[key] = value; - } - process.stdout.write( - binUtils.outputFormatter({ - type: 'json', - data: data, - }), - ); + process.stdout.write( + binUtils.outputFormatter({ + type: 'raw', + data, + }), + ); + } + break; + case 'json': + { + const data = {}; + for (const [key, value] of Object.entries(envp)) { + data[key] = value; } - break; - default: - utils.never(); - } + process.stdout.write( + binUtils.outputFormatter({ + type: 'json', + data: data, + }), + ); + } + break; + default: + utils.never(); } - } finally { - if (pkClient! != null) await pkClient.stop(); } - }, - ); + } finally { + if (pkClient! != null) await pkClient.stop(); + } + }); } } diff --git a/src/utils/options.ts b/src/utils/options.ts index d50e7330..37fd04aa 100644 --- a/src/utils/options.ts +++ b/src/utils/options.ts @@ -319,7 +319,7 @@ const parents = new commander.Option( ).default(false); const preserveNewline = new commander.Option( - '-pn --preserve-newline ', + '--preserve-newline ', 'Preserve the last trailing newline for the secret content', ) .argParser((value: string, previous: Array<[string, string?, string?]>) => { diff --git a/src/utils/parsers.ts b/src/utils/parsers.ts index 8015d53a..2b461292 100644 --- a/src/utils/parsers.ts +++ b/src/utils/parsers.ts @@ -196,38 +196,6 @@ const parsePort: (data: string) => Port = validateParserToArgParser( const parseSeedNodes: (data: string) => [SeedNodes, boolean] = validateParserToArgParser(nodesUtils.parseSeedNodes); -/** - * This parses the arguments used for the env command. It should be formatted as - * [-- cmd [cmdArgs...]] - * The cmd part of the list is separated by using `--`. - */ -function parseEnvArgs( - value: string, - prev: [Array, Array, boolean] | undefined, -): [Array, Array, boolean] { - const current: [Array, Array, boolean] = - prev ?? [[], [], false]; - const [secretsList, commandList, parsingCommandCurrent] = current; - let parsingCommand = parsingCommandCurrent; - if (!parsingCommand) { - // Parse a secret path - if (value !== '--') { - secretsList.push(parseSecretPathEnv(value)); - } else { - parsingCommand = true; - } - } else { - // Otherwise we just have the cmd args - commandList.push(value); - } - if (secretsList.length === 0 && commandList.length > 0) { - throw new commander.InvalidArgumentError( - 'You must provide at least 1 secret path', - ); - } - return [secretsList, commandList, parsingCommand]; -} - export { vaultNameRegex, secretPathRegex, @@ -256,5 +224,4 @@ export { parseProviderId, parseIdentityId, parseProviderIdList, - parseEnvArgs, }; diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index 5f6682ca..bcac1a5f 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -1,5 +1,4 @@ import type { VaultName } from 'polykey/dist/vaults/types'; -import type { ParsedSecretPathValue } from '@/types'; import path from 'path'; import fs from 'fs'; import fc from 'fast-check'; @@ -9,7 +8,6 @@ import PolykeyAgent from 'polykey/dist/PolykeyAgent'; import { vaultOps } from 'polykey/dist/vaults'; import * as keysUtils from 'polykey/dist/keys/utils'; import { sysexits } from 'polykey/dist/utils'; -import * as binParsers from '@/utils/parsers'; import * as testUtils from '../utils'; describe('commandEnv', () => { @@ -62,7 +60,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:SECRET`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -90,7 +87,6 @@ describe('commandEnv', () => { `${vaultName}:SECRET1`, `${vaultName}:SECRET2`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -120,7 +116,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:dir1`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -148,7 +143,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:SECRET=SECRET_NEW`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -177,7 +171,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:dir1=SECRET_NEW`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -211,7 +204,6 @@ describe('commandEnv', () => { `${vaultName}:SECRET2`, `${vaultName}:dir1`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -240,7 +232,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:SECRET1`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -277,7 +268,6 @@ describe('commandEnv', () => { `${vaultName}:SECRET3=SECRET4`, `${vaultName}:dir1`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -695,7 +685,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:${secretName}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -726,7 +715,6 @@ describe('commandEnv', () => { 'unix', `${vaultName}:${secretName}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -761,7 +749,6 @@ describe('commandEnv', () => { `${vaultName}:${secretName}`, `${vaultName}:${secretName}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -799,7 +786,6 @@ describe('commandEnv', () => { '--preserve-newline', `${vaultName}:${secretName1}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -838,7 +824,6 @@ describe('commandEnv', () => { '--preserve-newline', `${vaultName}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -865,7 +850,6 @@ describe('commandEnv', () => { '--preserve-newline', `${vaultName}`, '--', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -879,35 +863,6 @@ describe('commandEnv', () => { expect(jsonOut2[secretName2]).toBe(secretContent2); expect(jsonOut2[secretName3]).toBe(secretContent3); }); - test.prop([ - testUtils.secretPathEnvArrayArb, - fc.string().noShrink(), - testUtils.cmdArgsArrayArb, - ])( - 'parse secrets env arguments', - async (secretPathEnvArray, cmd, cmdArgsArray) => { - let output: - | [Array, Array, boolean] - | undefined = undefined; - // By running the parser directly, we are bypassing commander, so it works - // with a single -- - const args: Array = [ - ...secretPathEnvArray, - '--', - cmd, - ...cmdArgsArray, - ]; - for (const arg of args) { - output = binParsers.parseEnvArgs(arg, output); - } - const [parsedEnvs, parsedArgs] = output!; - const expectedSecretPathArray = secretPathEnvArray.map((v) => { - return binParsers.parseSecretPath(v); - }); - expect(parsedEnvs).toMatchObject(expectedSecretPathArray); - expect(parsedArgs).toMatchObject([cmd, ...cmdArgsArray]); - }, - ); test('handles no arguments', async () => { const command = ['secrets', 'env', '-np', dataDir, '--env-format', 'unix']; const result = await testUtils.pkExec(command, {