From 4e2cfcbcca25d3b879e85305a9963c4d0a8c1bc9 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 20 Jun 2024 10:43:45 +1000 Subject: [PATCH 1/8] fix: using single quotes `'` for `unix` format [ci skip] --- src/secrets/CommandEnv.ts | 2 +- tests/secrets/env.test.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index bcc3d376..de75d18f 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -239,7 +239,7 @@ class CommandEnv extends CommandPolykey { let data = ''; for (const [key, value] of Object.entries(envp)) { data += `# ${envpPath[key].nameOrId}:${envpPath[key].secretName}\n`; - data += `${key}="${value}"\n`; + data += `${key}='${value}'\n`; } process.stdout.write( binUtils.outputFormatter({ diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index 0640bac9..ad95629f 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -338,10 +338,10 @@ describe('commandEnv', () => { env: { PK_PASSWORD: password }, }); expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('SECRET1="this is the secret1"'); - expect(result.stdout).toContain('SECRET2="this is the secret2"'); - expect(result.stdout).toContain('SECRET3="this is the secret3"'); - expect(result.stdout).toContain('SECRET4="this is the secret4"'); + expect(result.stdout).toContain("SECRET1='this is the secret1'"); + expect(result.stdout).toContain("SECRET2='this is the secret2'"); + expect(result.stdout).toContain("SECRET3='this is the secret3'"); + expect(result.stdout).toContain("SECRET4='this is the secret4'"); }); test('should output unix format', async () => { const vaultId1 = await polykeyAgent.vaultManager.createVault( @@ -380,13 +380,13 @@ describe('commandEnv', () => { }); expect(result.exitCode).toBe(0); expect(result.stdout).toContain('# vault1:SECRET1'); - expect(result.stdout).toContain('SECRET1="this is the secret1"'); expect(result.stdout).toContain('# vault2:SECRET2'); - expect(result.stdout).toContain('SECRET2="this is the secret2"'); expect(result.stdout).toContain('# vault1:dir1/SECRET3'); - expect(result.stdout).toContain('SECRET3="this is the secret3"'); expect(result.stdout).toContain('# vault2:dir1/SECRET4'); - expect(result.stdout).toContain('SECRET4="this is the secret4"'); + expect(result.stdout).toContain("SECRET1='this is the secret1'"); + expect(result.stdout).toContain("SECRET2='this is the secret2'"); + expect(result.stdout).toContain("SECRET3='this is the secret3'"); + expect(result.stdout).toContain("SECRET4='this is the secret4'"); }); test('should output cmd format', async () => { const vaultId1 = await polykeyAgent.vaultManager.createVault( From 912d42e66355c60143bd910c11e99214dede01f0 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 20 Jun 2024 10:45:11 +1000 Subject: [PATCH 2/8] fix: removed path comments from the formatted outputs [ci skip] --- src/secrets/CommandEnv.ts | 154 ++++++++++++++++++-------------------- tests/secrets/env.test.ts | 12 --- 2 files changed, 74 insertions(+), 92 deletions(-) diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index de75d18f..6c030321 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -101,95 +101,92 @@ class CommandEnv extends CommandPolykey { }); // Getting envs - const [envp, envpPath] = 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, - secretName, - metadata: first ? auth : undefined, - }); - first = false; - } - await writer.close(); - })(); + 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, + 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(); } } - envp[newName] = secretContent; - envpPath[newName] = { - nameOrId, - secretName, - }; + newName = secretEnvName; } - await writeP; - return [envp, envpPath]; - }, - meta, - ); + // 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(); + } + } + envp[newName] = secretContent; + envpPath[newName] = { + nameOrId, + secretName, + }; + } + await writeP; + return [envp, envpPath]; + }, meta); // End connection early to avoid errors on server await pkClient.stop(); @@ -238,7 +235,6 @@ class CommandEnv extends CommandPolykey { // Formatting as a .env file let data = ''; for (const [key, value] of Object.entries(envp)) { - data += `# ${envpPath[key].nameOrId}:${envpPath[key].secretName}\n`; data += `${key}='${value}'\n`; } process.stdout.write( @@ -254,7 +250,6 @@ class CommandEnv extends CommandPolykey { // Formatting as a .bat file for windows cmd let data = ''; for (const [key, value] of Object.entries(envp)) { - data += `REM ${envpPath[key].nameOrId}:${envpPath[key].secretName}\n`; data += `set "${key}=${value}"\n`; } process.stdout.write( @@ -270,7 +265,6 @@ class CommandEnv extends CommandPolykey { // Formatting as a .bat file for windows cmd let data = ''; for (const [key, value] of Object.entries(envp)) { - data += `# ${envpPath[key].nameOrId}:${envpPath[key].secretName}\n`; data += `\$env:${key} = '${value}'\n`; } process.stdout.write( diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index ad95629f..92651f49 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -379,10 +379,6 @@ describe('commandEnv', () => { env: { PK_PASSWORD: password }, }); expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('# vault1:SECRET1'); - expect(result.stdout).toContain('# vault2:SECRET2'); - expect(result.stdout).toContain('# vault1:dir1/SECRET3'); - expect(result.stdout).toContain('# vault2:dir1/SECRET4'); expect(result.stdout).toContain("SECRET1='this is the secret1'"); expect(result.stdout).toContain("SECRET2='this is the secret2'"); expect(result.stdout).toContain("SECRET3='this is the secret3'"); @@ -424,13 +420,9 @@ describe('commandEnv', () => { env: { PK_PASSWORD: password }, }); expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('REM vault1:SECRET1'); expect(result.stdout).toContain('set "SECRET1=this is the secret1"'); - expect(result.stdout).toContain('REM vault2:SECRET2'); expect(result.stdout).toContain('set "SECRET2=this is the secret2"'); - expect(result.stdout).toContain('REM vault1:dir1/SECRET3'); expect(result.stdout).toContain('set "SECRET3=this is the secret3"'); - expect(result.stdout).toContain('REM vault2:dir1/SECRET4'); expect(result.stdout).toContain('set "SECRET4=this is the secret4"'); }); test('should output powershell format', async () => { @@ -469,13 +461,9 @@ describe('commandEnv', () => { env: { PK_PASSWORD: password }, }); expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('# vault1:SECRET1'); expect(result.stdout).toContain(`$env:SECRET1 = 'this is the secret1'`); - expect(result.stdout).toContain('# vault2:SECRET2'); expect(result.stdout).toContain(`$env:SECRET2 = 'this is the secret2'`); - expect(result.stdout).toContain('# vault1:dir1/SECRET3'); expect(result.stdout).toContain(`$env:SECRET3 = 'this is the secret3'`); - expect(result.stdout).toContain('# vault2:dir1/SECRET4'); expect(result.stdout).toContain(`$env:SECRET4 = 'this is the secret4'`); }); test('should output json format', async () => { From dc67548b24a2921870b90916b13397ce7b9f9c79 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 20 Jun 2024 12:37:24 +1000 Subject: [PATCH 3/8] tests: added test for checking if newlines are unmodified by the env command --- tests/secrets/env.test.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index 92651f49..84f0f859 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -750,4 +750,37 @@ describe('commandEnv', () => { expect(result.exitCode).toBe(0); expect(result.stdout).toInclude('this is a secret2'); }); + test('newlines in secrets are untouched', async () => { + const vaultId = await polykeyAgent.vaultManager.createVault(vaultName); + + await polykeyAgent.vaultManager.withVaults([vaultId], async (vault) => { + await vaultOps.addSecret( + vault, + 'SECRET', + 'this is a secret\nit has multiple lines\n', + ); + }); + + command = [ + 'secrets', + 'env', + '-np', + dataDir, + '-e', + `${vaultName}:SECRET`, + '--env-format', + 'unix', + '--', + 'node', + '-e', + 'console.log(JSON.stringify(process.env))', + ]; + + const result = await testUtils.pkExec([...command], { + env: { PK_PASSWORD: password }, + }); + expect(result.exitCode).toBe(0); + const jsonOut = JSON.parse(result.stdout); + expect(jsonOut['SECRET']).toBe('this is a secret\nit has multiple lines\n'); + }); }); From aa87b95e4064f65f51e5728c198c6f7706e72e86 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 20 Jun 2024 13:21:12 +1000 Subject: [PATCH 4/8] test: checking ci --- tests/secrets/env.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index 84f0f859..dd86e026 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -784,3 +784,5 @@ describe('commandEnv', () => { expect(jsonOut['SECRET']).toBe('this is a secret\nit has multiple lines\n'); }); }); + +// TODO: wip From 7322f5ac6bb9322762a4713aa40b1ddb46a3369c Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 24 Jun 2024 15:09:15 +1000 Subject: [PATCH 5/8] fix: aligned `secrets env` arguments with documentation Rather than specifying `-e ` as a required option. You provide the secretPaths as part of the main argument now. So we provide `secrets env vault:path command arg1 arg2`. The parser expects the secret paths first. Any argument that doesn't match the secret path will be taken as part of the command args. All args are taken as command args after the first non secret path arg. [ci skip] --- npmDepsHash | 2 +- package-lock.json | 33 ++++++++++++++ package.json | 1 + src/secrets/CommandEnv.ts | 15 +++---- src/utils/parsers.ts | 47 +++++++++++++++++--- tests/secrets/env.test.ts | 92 +++++++++++++++++++-------------------- tests/utils/utils.ts | 24 +++++++++- 7 files changed, 150 insertions(+), 64 deletions(-) diff --git a/npmDepsHash b/npmDepsHash index 97135837..424387b1 100644 --- a/npmDepsHash +++ b/npmDepsHash @@ -1 +1 @@ -sha256-KFydumA3XA3dLkx+27/XVAZ79+ISJf4LpESBOh9qGEI= +sha256-7poehWX7GVq/iWVeP6Yk4Yhk+Xii9NSEp6tJCHqsZr0= diff --git a/package-lock.json b/package-lock.json index 0610afc2..3e978213 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "polykey": "dist/polykey.js" }, "devDependencies": { + "@fast-check/jest": "^1.1.0", "@matrixai/errors": "^1.2.0", "@matrixai/exec": "^0.1.4", "@matrixai/logger": "^3.1.0", @@ -830,6 +831,38 @@ "node": "^12.22.0 || ^14.17.0 || >=16.0.0" } }, + "node_modules/@fast-check/jest": { + "version": "1.8.2", + "resolved": "https://registry.npmjs.org/@fast-check/jest/-/jest-1.8.2.tgz", + "integrity": "sha512-+UgQKZ0og0olUZXWgZ5Zcw42eN+3OB0Nfw0CU9OnlHBhHFnd8xppUYviX5HriAyUsAko1t/li5LZ9mSIIakmhg==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "dependencies": { + "fast-check": "^3.0.0" + }, + "peerDependencies": { + "@fast-check/worker": ">=0.0.7 <0.5.0", + "@jest/expect": ">=28.0.0", + "@jest/globals": ">=25.5.2" + }, + "peerDependenciesMeta": { + "@fast-check/worker": { + "optional": true + }, + "@jest/expect": { + "optional": true + } + } + }, "node_modules/@humanwhocodes/config-array": { "version": "0.11.11", "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.11.tgz", diff --git a/package.json b/package.json index 56254a85..11c5cc22 100644 --- a/package.json +++ b/package.json @@ -124,6 +124,7 @@ "@matrixai/errors": "^1.2.0", "@matrixai/logger": "^3.1.0", "@matrixai/exec": "^0.1.4", + "@fast-check/jest": "^1.1.0", "@swc/core": "1.3.82", "@swc/jest": "^0.2.29", "@types/jest": "^29.5.2", diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index 6c030321..1dfb6dc0 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -42,26 +42,25 @@ class CommandEnv extends CommandPolykey { this.addOption(binOptions.nodeId); this.addOption(binOptions.clientHost); this.addOption(binOptions.clientPort); - this.addOption(binOptions.envVariables); this.addOption(binOptions.envFormat); this.addOption(binOptions.envInvalid); this.addOption(binOptions.envDuplicate); - this.argument('[cmd] [argv...]', 'command and arguments'); + this.argument('[args...]', 'command and arguments formatted as [envPaths...][cmd][cmdArgs...]'); this.addHelpText('after', helpText); this.action(async (args: Array, options) => { - const [cmd, ...argv] = args; + const { default: PolykeyClient } = await import( + 'polykey/dist/PolykeyClient' + ); + const binParsers = await import('../utils/parsers'); const { - env: envVariables, envInvalid, envDuplicate, envFormat, }: { - env: Array<[string, string, string?]>; envInvalid: 'error' | 'warn' | 'ignore'; envDuplicate: 'keep' | 'overwrite' | 'warn' | 'error'; envFormat: 'auto' | 'unix' | 'cmd' | 'powershell' | 'json'; } = options; - // There are a few stages here // 1. parse the desired secrets // 2. obtain the desired secrets @@ -69,9 +68,7 @@ class CommandEnv extends CommandPolykey { // a. exec the command with the provided env variables from the secrets // b. output the env variables in the desired format - const { default: PolykeyClient } = await import( - 'polykey/dist/PolykeyClient' - ); + const [envVariables, [cmd, ...argv]] = binParsers.parseEnvArgs(args); const clientOptions = await binProcessors.processClientOptions( options.nodePath, options.nodeId, diff --git a/src/utils/parsers.ts b/src/utils/parsers.ts index 0151da40..8ff43c86 100644 --- a/src/utils/parsers.ts +++ b/src/utils/parsers.ts @@ -8,6 +8,11 @@ import * as gestaltsUtils from 'polykey/dist/gestalts/utils'; import * as networkUtils from 'polykey/dist/network/utils'; import * as nodesUtils from 'polykey/dist/nodes/utils'; +const secretPathRegex = + /^([\w-]+)(?::)([\w\-\\\/\.\$]+)(?:=)?([a-zA-Z_][\w]+)?$/; +const secretPathEnvRegex = + /^([\w-]+)(?::)([\w\-\\\/\.\$]+)(?:=)?([a-zA-Z_]+[a-zA-Z0-9_]*)?$/; + /** * Converts a validation parser to commander argument parser */ @@ -64,8 +69,6 @@ function parseCoreCount(v: string): number | undefined { function parseSecretPath(secretPath: string): [string, string, string?] { // E.g. If 'vault1:a/b/c', ['vault1', 'a/b/c'] is returned // If 'vault1:a/b/c=VARIABLE', ['vault1, 'a/b/c', 'VARIABLE'] is returned - const secretPathRegex = - /^([\w-]+)(?::)([\w\-\\\/\.\$]+)(?:=)?([a-zA-Z_][\w]+)?$/; if (!secretPathRegex.test(secretPath)) { throw new commander.InvalidArgumentError( `${secretPath} is not of the format :`, @@ -80,16 +83,13 @@ function parseEnvPath(secretPath: string): [string, string, string?] { // E.g. If 'vault1:a/b/c', ['vault1', 'a/b/c'] is returned // If 'vault1:a/b/c=VARIABLE', ['vault1, 'a/b/c', 'VARIABLE'] is returned // VARIABLE must be a valid ENV variable name - const secretPathRegex = - /^([\w-]+)(?::)([\w\-\\\/\.\$]+)(?:=)?([a-zA-Z_]+[a-zA-Z0-9_]*)?$/; - // /^([\w-]+)(?::)([\w\-\\\/\.\$]+)(?:=)?([a-zA-Z_][\w]+)?$/; - if (!secretPathRegex.test(secretPath)) { + if (!secretPathEnvRegex.test(secretPath)) { throw new commander.InvalidArgumentError( `${secretPath} is not of the format :`, ); } const [, vaultName, directoryPath, value] = - secretPath.match(secretPathRegex)!; + secretPath.match(secretPathEnvRegex)!; return [vaultName, directoryPath, value]; } @@ -143,7 +143,39 @@ 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 in two ways, either the user explicitly uses `--` or the first non-secret path separates it. + */ +function parseEnvArgs( + args: Array, +): [Array<[string, string, string?]>, Array] { + let isEnvStage = true; + const envPaths: Array<[string, string, string?]> = []; + const cmdArgs: Array = []; + for (const arg of args) { + if (isEnvStage) { + // Parse a secret path + try { + envPaths.push(parseEnvPath(arg)); + } catch (e) { + if (!(e instanceof commander.InvalidArgumentError)) throw e; + // If we get an invalid argument error then we switch over to parsing args verbatim + cmdArgs.push(arg); + isEnvStage = false; + } + } else { + // Otherwise we just have the cmd args + cmdArgs.push(arg); + } + } + return [envPaths, cmdArgs]; +} + export { + secretPathRegex, + secretPathEnvRegex, validateParserToArgParser, validateParserToArgListParser, parseCoreCount, @@ -164,4 +196,5 @@ export { parseProviderId, parseIdentityId, parseProviderIdList, + parseEnvArgs, }; diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index dd86e026..a1127a3f 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -1,11 +1,14 @@ import type { VaultName } from 'polykey/dist/vaults/types'; import path from 'path'; import fs from 'fs'; +import fc from 'fast-check'; +import { test } from '@fast-check/jest'; 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 { sysexits } from 'polykey/dist/utils'; +import * as binParsers from '@/utils/parsers'; import * as testUtils from '../utils'; describe('commandEnv', () => { @@ -55,11 +58,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET`, '--env-format', 'unix', '--', + `${vaultName}:SECRET`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -85,12 +87,11 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET1`, - `${vaultName}:SECRET2`, '--env-format', 'unix', '--', + `${vaultName}:SECRET1`, + `${vaultName}:SECRET2`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -119,11 +120,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:dir1`, '--env-format', 'unix', '--', + `${vaultName}:dir1`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -150,11 +150,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET=SECRET_NEW`, '--env-format', 'unix', '--', + `${vaultName}:SECRET=SECRET_NEW`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -182,11 +181,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:dir1=SECRET_NEW`, '--env-format', 'unix', '--', + `${vaultName}:dir1=SECRET_NEW`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -217,13 +215,12 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET1`, - `${vaultName}:SECRET2`, - `${vaultName}:dir1`, '--env-format', 'unix', '--', + `${vaultName}:SECRET1`, + `${vaultName}:SECRET2`, + `${vaultName}:dir1`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -251,11 +248,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET1`, '--env-format', 'unix', '--', + `${vaultName}:SECRET1`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -288,14 +284,13 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', + '--env-format', + 'unix', + '--', `${vaultName}:SECRET1`, `${vaultName}:SECRET2=SECRET1`, `${vaultName}:SECRET3=SECRET4`, `${vaultName}:dir1`, - '--env-format', - 'unix', - '--', 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -328,10 +323,9 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:.`, '--env-format', 'unix', + `${vaultName}:.`, ]; const result = await testUtils.pkExec([...command], { @@ -368,11 +362,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}1:.`, - `${vaultName}2:.`, '--env-format', 'unix', + `${vaultName}1:.`, + `${vaultName}2:.`, ]; const result = await testUtils.pkExec([...command], { @@ -409,11 +402,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}1:.`, - `${vaultName}2:.`, '--env-format', 'cmd', + `${vaultName}1:.`, + `${vaultName}2:.`, ]; const result = await testUtils.pkExec([...command], { @@ -450,11 +442,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}1:.`, - `${vaultName}2:.`, '--env-format', 'powershell', + `${vaultName}1:.`, + `${vaultName}2:.`, ]; const result = await testUtils.pkExec([...command], { @@ -482,10 +473,9 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:.`, '--env-format', 'json', + `${vaultName}:.`, ]; const result = await testUtils.pkExec([...command], { @@ -528,7 +518,6 @@ describe('commandEnv', () => { dataDir, '--env-format', 'unix', - '-e', ...valid.map((v) => `${vaultName}:SECRET=${v}`), ], { env: { PK_PASSWORD: password } }, @@ -571,7 +560,6 @@ describe('commandEnv', () => { 'unix', '-ei', 'error', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -596,7 +584,6 @@ describe('commandEnv', () => { 'unix', '-ei', 'warn', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -625,7 +612,6 @@ describe('commandEnv', () => { 'unix', '-ei', 'ignore', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -656,7 +642,6 @@ describe('commandEnv', () => { 'unix', '-ed', 'error', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -684,7 +669,6 @@ describe('commandEnv', () => { 'unix', '-ed', 'warn', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -714,7 +698,6 @@ describe('commandEnv', () => { 'unix', '-ed', 'keep', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -742,7 +725,6 @@ describe('commandEnv', () => { 'unix', '-ed', 'overwrite', - '-e', `${vaultName}:.`, ], { env: { PK_PASSWORD: password } }, @@ -766,11 +748,10 @@ describe('commandEnv', () => { 'env', '-np', dataDir, - '-e', - `${vaultName}:SECRET`, '--env-format', 'unix', '--', + `${vaultName}:SECRET`, 'node', '-e', 'console.log(JSON.stringify(process.env))', @@ -783,6 +764,25 @@ describe('commandEnv', () => { const jsonOut = JSON.parse(result.stdout); expect(jsonOut['SECRET']).toBe('this is a secret\nit has multiple lines\n'); }); + test.prop([ + testUtils.secretPathEnvArrayArb, + fc.string().noShrink(), + testUtils.cmdArgsArrayArb, + ])( + 'parse secrets env arguments', + async (secretPathEnvArray, cmd, cmdArgsArray) => { + // If we don't use the optional `--` delimiter then we can't include `:` in vault names + fc.pre(!cmd.includes(':')); + + const args: Array = [...secretPathEnvArray, cmd, ...cmdArgsArray]; + const parsed = binParsers.parseEnvArgs(args); + const [parsedEnvs, parsedArgs] = parsed; + const expectedSecretPathArray = secretPathEnvArray.map((v) => { + const matched = v.match(binParsers.secretPathEnvRegex)!; + return [matched[1], matched[2], matched[3]]; + }); + expect(parsedEnvs).toMatchObject(expectedSecretPathArray); + expect(parsedArgs).toMatchObject([cmd, ...cmdArgsArray]); + }, + ); }); - -// TODO: wip diff --git a/tests/utils/utils.ts b/tests/utils/utils.ts index cd638469..84bb2553 100644 --- a/tests/utils/utils.ts +++ b/tests/utils/utils.ts @@ -1,5 +1,7 @@ import type PolykeyAgent from 'polykey/dist/PolykeyAgent'; import { promise } from 'polykey/dist/utils/utils'; +import fc from 'fast-check'; +import * as binParsers from '@/utils/parsers'; function testIf(condition: boolean) { return condition ? test : test.skip; @@ -86,4 +88,24 @@ async function nodesConnect(localNode: PolykeyAgent, remoteNode: PolykeyAgent) { ); } -export { testIf, describeIf, trackTimers, nodesConnect }; +const secretPathEnvArb = fc + .stringMatching(binParsers.secretPathEnvRegex) + .noShrink(); +const secretPathEnvArrayArb = fc + .array(secretPathEnvArb, { minLength: 1, size: 'small' }) + .noShrink(); +const cmdArgsArrayArb = fc + .array(fc.oneof(fc.string(), secretPathEnvArb, fc.constant('--')), { + size: 'small', + }) + .noShrink(); + +export { + testIf, + describeIf, + trackTimers, + nodesConnect, + secretPathEnvArb, + secretPathEnvArrayArb, + cmdArgsArrayArb, +}; From cb261ab71b68f00927f1c93bdc6726ba39f95c44 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 24 Jun 2024 16:25:07 +1000 Subject: [PATCH 6/8] fix: secrets env command should properly handle no arguments or no secrets [ci skip] --- src/secrets/CommandEnv.ts | 10 +++++++--- src/utils/parsers.ts | 35 +++++++++++++++++++++++++++++++++++ tests/secrets/env.test.ts | 24 ++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index 1dfb6dc0..31dcb6da 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -7,6 +7,7 @@ import * as binUtils from '../utils'; import * as binErrors from '../errors'; import CommandPolykey from '../CommandPolykey'; import * as binOptions from '../utils/options'; +import * as binParsers from '../utils/parsers'; const description = ` Run a command with the given secrets and env variables. If no command is specified then the variables are printed to stdout in the format specified by env-format. @@ -45,13 +46,16 @@ class CommandEnv extends CommandPolykey { this.addOption(binOptions.envFormat); this.addOption(binOptions.envInvalid); this.addOption(binOptions.envDuplicate); - this.argument('[args...]', 'command and arguments formatted as [envPaths...][cmd][cmdArgs...]'); + this.argument( + '', + 'command and arguments formatted as [envPaths...][cmd][cmdArgs...]', + binParsers.testParse, + ); this.addHelpText('after', helpText); this.action(async (args: Array, options) => { const { default: PolykeyClient } = await import( 'polykey/dist/PolykeyClient' ); - const binParsers = await import('../utils/parsers'); const { envInvalid, envDuplicate, @@ -68,7 +72,7 @@ class CommandEnv extends CommandPolykey { // 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]] = binParsers.parseEnvArgs(args); + const [envVariables, [cmd, ...argv]] = args; const clientOptions = await binProcessors.processClientOptions( options.nodePath, options.nodeId, diff --git a/src/utils/parsers.ts b/src/utils/parsers.ts index 8ff43c86..487b49d2 100644 --- a/src/utils/parsers.ts +++ b/src/utils/parsers.ts @@ -170,9 +170,43 @@ function parseEnvArgs( cmdArgs.push(arg); } } + if (envPaths.length === 0) { + throw new commander.InvalidArgumentError( + 'You must provide at least 1 secret path', + ); + } return [envPaths, cmdArgs]; } +function testParse( + value: string, + prev: [Array<[string, string, string?]>, Array], +): [Array<[string, string, string?]>, Array] { + const current: [Array<[string, string, string?]>, Array] = prev ?? [ + [], + [], + ]; + if (current[1].length === 0) { + // Parse a secret path + try { + current[0].push(parseEnvPath(value)); + } catch (e) { + if (!(e instanceof commander.InvalidArgumentError)) throw e; + // If we get an invalid argument error then we switch over to parsing args verbatim + current[1].push(value); + } + } else { + // Otherwise we just have the cmd args + current[1].push(value); + } + if (current[0].length === 0 && current[1].length > 0) { + throw new commander.InvalidArgumentError( + 'You must provide at least 1 secret path', + ); + } + return current; +} + export { secretPathRegex, secretPathEnvRegex, @@ -197,4 +231,5 @@ export { parseIdentityId, parseProviderIdList, parseEnvArgs, + testParse, }; diff --git a/tests/secrets/env.test.ts b/tests/secrets/env.test.ts index a1127a3f..32e9f26e 100644 --- a/tests/secrets/env.test.ts +++ b/tests/secrets/env.test.ts @@ -785,4 +785,28 @@ describe('commandEnv', () => { expect(parsedArgs).toMatchObject([cmd, ...cmdArgsArray]); }, ); + test('handles no arguments', async () => { + command = ['secrets', 'env', '-np', dataDir, '--env-format', 'unix']; + + const result1 = await testUtils.pkExec([...command], { + env: { PK_PASSWORD: password }, + }); + expect(result1.exitCode).toBe(64); + }); + test('Handles providing no secret paths', async () => { + command = [ + 'secrets', + 'env', + '-np', + dataDir, + '--env-format', + 'unix', + 'someCommand', + ]; + + const result1 = await testUtils.pkExec([...command], { + env: { PK_PASSWORD: password }, + }); + expect(result1.exitCode).toBe(64); + }); }); From 5ec4ca86f20da40fed797f0fc4a372c381302001 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 25 Jun 2024 11:13:42 +1000 Subject: [PATCH 7/8] fix: cut out a lot of unnecessary help text [ci skip] --- src/secrets/CommandEnv.ts | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/src/secrets/CommandEnv.ts b/src/secrets/CommandEnv.ts index 31dcb6da..88de2ca6 100644 --- a/src/secrets/CommandEnv.ts +++ b/src/secrets/CommandEnv.ts @@ -9,37 +9,13 @@ import CommandPolykey from '../CommandPolykey'; import * as binOptions from '../utils/options'; import * as binParsers from '../utils/parsers'; -const description = ` -Run a command with the given secrets and env variables. If no command is specified then the variables are printed to stdout in the format specified by env-format. - -When selecting secrets with --env secrets with invalid names can be selected. By default when these are encountered then the command will throw an error. This behaviour can be modified with '--env-invalid'. the invalid name can be silently dropped with 'ignore' or logged out with 'warn' - -Duplicate secret names can be specified, by default with 'overwrite' the env variable will be overwritten with the latest found secret of that name. It can be specified to 'keep' the first found secret of that name, 'error' to throw if there is a duplicate and 'warn' to log a warning while overwriting. -`; - -const helpText = ` -This command has two main ways of functioning. Executing a provided command or outputting formatted env variables to] stdout. - -Running the command with 'polykey secrets env --env vault:secret -- some command' will do process replacement to run 'some command' while providing environment variables selected by '-e' to that process. Note that process replacement is only supported on unix systems such as linux or macos. When running on windows a child process will be used. - -Running the command with 'polykey secrets env --env vault:secret --env-format ' will output the environment variables to stdout with the given . The following formats are supported, 'auto', 'json', 'unix', 'cmd' and 'powershell'. - -'auto' will automatically detect the current platform and select the appropriate format. This is 'unix' for unix based systems and 'cmd' for windows. - -'json' Will format the environment variables as a json object in the form {'key': 'value'}. - -'unix' Will format the environment variables as a '.env' file for use on unix systems. It will include comments before each variable showing the secret path used for that variable. - -'cmd' Will format the environment variables as a '.bat' file for use on windows cmd. It will include comments before each variable showing the secret path used for that variable. - -'powershell' Will format the environment variables as a '.ps1' file for use on windows Powershell. It will include comments before each variable showing the secret path used for that variable. -`; - class CommandEnv extends CommandPolykey { constructor(...args: ConstructorParameters) { super(...args); this.name('env'); - this.description(description); + this.description( + `Run a command with the given secrets and env variables using process replacement. If no command is specified then the variables are printed to stdout in the format specified by env-format.`, + ); this.addOption(binOptions.nodeId); this.addOption(binOptions.clientHost); this.addOption(binOptions.clientPort); @@ -51,7 +27,6 @@ class CommandEnv extends CommandPolykey { 'command and arguments formatted as [envPaths...][cmd][cmdArgs...]', binParsers.testParse, ); - this.addHelpText('after', helpText); this.action(async (args: Array, options) => { const { default: PolykeyClient } = await import( 'polykey/dist/PolykeyClient' From 9768aaa7db5c6584080e909a2ed6deb05ce3ad2c Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Fri, 28 Jun 2024 16:47:45 +1000 Subject: [PATCH 8/8] chore: updating deps hash [ci skip] --- npmDepsHash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npmDepsHash b/npmDepsHash index 424387b1..0b05c766 100644 --- a/npmDepsHash +++ b/npmDepsHash @@ -1 +1 @@ -sha256-7poehWX7GVq/iWVeP6Yk4Yhk+Xii9NSEp6tJCHqsZr0= +sha256-eDUzTAx7aYrCZinucP9pNHaayK66WNiPz3vOs9H5xNI=