diff --git a/src/secrets/CommandGet.ts b/src/secrets/CommandGet.ts index e2e72047..917edd88 100644 --- a/src/secrets/CommandGet.ts +++ b/src/secrets/CommandGet.ts @@ -58,10 +58,12 @@ class CommandGet extends CommandPolykey { }), meta, ); - const secretContent = Buffer.from(response.secretContent, 'binary'); + const secretContent = response.secretContent; const outputFormatted = binUtils.outputFormatter({ type: 'raw', - data: `"${binUtils.encodeQuotes(secretContent.toString('utf-8'))}"`, // Encodes any secret content with escape characters + data: binUtils.encodeEscapedWrapped(secretContent) + ? binUtils.encodeEscaped(secretContent) + : secretContent, }); process.stdout.write(outputFormatted); } finally { diff --git a/src/utils/utils.ts b/src/utils/utils.ts index a9b3fa52..4854d1c1 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -50,7 +50,7 @@ type OutputObject = data: Error; }; -function standardErrorReplacer(key: string, value: any) { +function standardErrorReplacer(_key: string, value: any) { if (value instanceof Error && !(value instanceof ErrorPolykey)) { return { type: value.name, @@ -64,35 +64,64 @@ function standardErrorReplacer(key: string, value: any) { return value; } +function encodeEscapedReplacer(_key: string, value: any) { + if (typeof value === 'string') { + return encodeEscaped(value); + } + return value; +} + /** - * This function calls encodeNonPrintable on substrings within the input enclosed with `""`. - * Any usage of `\\"` within `""` will escape it. + * This function: * - * @param str - * @see {@link encodeNonPrintable} - * @returns + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\n \\r \\t to escaped versions, \\\\n \\\\r and \\\\t. + * 3. Converts other control characters to their Unicode escape sequences. + * 4. Converts ' \` " to escaped versions, \\\\' \\\\\` and \\\\" + * 5. Wraps the whole thing in `""` if any characters have been encoded. */ -function encodeWrappedStrings(str: string) { - return str.replace(/"(\\.|[^"])*"/g, encodeNonPrintable); +function encodeEscapedWrapped(str: string): string { + if (!encodeEscapedRegex.test(str)) { + return str; + } + return `"${encodeEscaped(str)}"`; } -function encodeQuotes(str: string): string { - return str.replace(/[`"']/g, (char) => `\\` + char); +/** + * This function: + * + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\\\n \\\\r and \\\\t to unescaped versions, \\n \\r \\t. + * 3. Converts Unicode escape sequences to their control characters. + * 4. Converts \\\\' \\\\\` and \\\\" to their unescaped versions, ' \` ". + * 5. If it is wrapped in "" double quotes, the double quotes will be trimmed. + */ +function decodeEscapedWrapped(str: string): string { + if (!decodeEscapedRegex.test(str)) { + return str; + } + return decodeEscaped(str.substring(1, str.length - 1)); } +// We want to actually match control codes here! +// eslint-disable-next-line no-control-regex +const encodeEscapedRegex = /[\x00-\x1F\x7F-\x9F"'`]/g; + /** * This function: + * * 1. Keeps regular spaces, only ' ', as they are. - * 2. Converts \n \r \t to escaped versions, \\n \\r and \\t. - * 3. Converts other control characters to their Unicode escape sequences. + * 2. Converts \\n \\r \\t to escaped versions, \\\\n \\\\r and \\\\t. + * 3. Converts other control characters to their Unicode escape sequences.\ + * 4. Converts ' \` " to escaped versions, \\\\' \\\\\` and \\\\" + * + * Unless you're using this in a `JSON.stringify` replacer, you probably want to use {@link encodeEscapedWrapped} instead. */ -function encodeNonPrintable(str: string): string { - // We want to actually match control codes here! - // eslint-disable-next-line no-control-regex - return str.replace(/[\x00-\x1F\x7F-\x9F]/g, (char) => { +function encodeEscaped(str: string): string { + return str.replace(encodeEscapedRegex, (char) => { switch (char) { case ' ': - return char; // Preserve regular space + return char; case '\n': return '\\n'; // Encode newline case '\r': @@ -103,6 +132,10 @@ function encodeNonPrintable(str: string): string { return '\\v'; // Encode tab case '\f': return '\\f'; // Encode tab + case '"': + case "'": + case '`': + return '\\' + char; // Add cases for other whitespace characters if needed default: // Return the Unicode escape sequence for control characters @@ -111,6 +144,49 @@ function encodeNonPrintable(str: string): string { }); } +const decodeEscapedRegex = /\\([nrtvf"'`]|u[0-9a-fA-F]{4})/g; + +/** + * This function: + * + * 1. Keeps regular spaces, only ' ', as they are. + * 2. Converts \\\\n \\\\r and \\\\t to unescaped versions, \\n \\r \\t. + * 3. Converts Unicode escape sequences to their control characters. + * 4. Converts \\\\' \\\\\` and \\\\" to their unescaped versions, ' \` ". + * + * Unless you're using this in a `JSON.parse` reviver, you probably want to use {@link decodeEscapedWrapped} instead. + */ +function decodeEscaped(str: string): string { + return str.replace(decodeEscapedRegex, (substr) => { + // Unicode escape sequence must be characters (e.g. `\u0000`) + if (substr.length === 6 && substr.at(1) === 'u') { + return String.fromCharCode(parseInt(substr.substring(2), 16)); + } + // Length of substr will always be at least 1 + const lastChar = substr.at(-1); + if (lastChar == null) { + utils.never(); + } + switch (lastChar) { + case 'n': + return '\n'; + case 'r': + return '\r'; + case 't': + return '\t'; + case 'v': + return '\v'; + case 'f': + return '\f'; + case '"': + case "'": + case '`': + return lastChar; + } + utils.never(); + }); +} + /** * Formats a message suitable for output. * @@ -119,26 +195,20 @@ function encodeNonPrintable(str: string): string { * @see {@link encodeWrappedStrings} for information regarding wrapping strings with `""` for encoding escaped characters * @returns */ -function outputFormatter(msg: OutputObject): string { - let data = msg.data; +function outputFormatter(msg: OutputObject): string | Uint8Array { switch (msg.type) { case 'raw': - if (ArrayBuffer.isView(data)) { - const td = new TextDecoder('utf-8'); - data = td.decode(data); - } - data = encodeWrappedStrings(data); - return data; + return msg.data; case 'list': - return outputFormatterList(data); + return outputFormatterList(msg.data); case 'table': - return outputFormatterTable(data, msg.options); + return outputFormatterTable(msg.data, msg.options); case 'dict': - return outputFormatterDict(data); + return outputFormatterDict(msg.data); case 'json': - return outputFormatterJson(data); + return outputFormatterJson(msg.data); case 'error': - return outputFormatterError(data); + return outputFormatterError(msg.data); } } @@ -146,7 +216,7 @@ function outputFormatterList(items: Array): string { let output = ''; for (const elem of items) { // Convert null or undefined to empty string - output += `${elem != null ? encodeWrappedStrings(elem) : ''}\n`; + output += `${elem ?? ''}\n`; } return output; } @@ -199,12 +269,10 @@ function outputFormatterTable( for (const column in options?.columns ?? row) { if (row[column] != null) { if (typeof row[column] === 'string') { - row[column] = encodeQuotes(row[column]); - row[column] = `"${row[column]}"`; + row[column] = encodeEscapedWrapped(row[column]); } else { - row[column] = JSON.stringify(row[column]); + row[column] = JSON.stringify(row[column], encodeEscapedReplacer); } - row[column] = encodeWrappedStrings(row[column]); } // Null or '' will both cause cellLength to be 3 const cellLength = @@ -271,12 +339,10 @@ function outputFormatterDict(data: POJO): string { } if (typeof value === 'string') { - value = encodeQuotes(value); - value = `"${value}"`; + value = encodeEscapedWrapped(value); } else { - value = JSON.stringify(value); + value = JSON.stringify(value, encodeEscapedReplacer); } - value = encodeWrappedStrings(value); value = value.replace(/(?:\r\n|\n)$/, ''); value = value.replace(/(\r\n|\n)/g, '$1\t'); @@ -415,6 +481,7 @@ function remoteErrorCause(e: any): [any, number] { export { verboseToLogLevel, standardErrorReplacer, + encodeEscapedReplacer, outputFormatter, outputFormatterList, outputFormatterTable, @@ -423,9 +490,12 @@ export { outputFormatterError, retryAuthentication, remoteErrorCause, - encodeWrappedStrings, - encodeQuotes, - encodeNonPrintable, + encodeEscapedWrapped, + encodeEscaped, + encodeEscapedRegex, + decodeEscapedWrapped, + decodeEscaped, + decodeEscapedRegex, }; export type { OutputObject }; diff --git a/src/vaults/CommandScan.ts b/src/vaults/CommandScan.ts index 191b958e..ab017da8 100644 --- a/src/vaults/CommandScan.ts +++ b/src/vaults/CommandScan.ts @@ -56,7 +56,7 @@ class CommandScan extends CommandPolykey { const permissions = vault.permissions.join(','); data.push( `${vaultName}${' '.repeat(4)}${vaultIdEncoded}${' '.repeat( - 5, + 4, )}${permissions}`, ); } diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 5b4d0158..7751dbe1 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -52,9 +52,7 @@ describe('bin/utils', () => { includeHeaders: true, }, }); - expect(tableOutput).toBe( - '"value1"\t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', - ); + expect(tableOutput).toBe('value1\tvalue2\ndata1 \tdata2\nN/A \tN/A\n'); // JSON const jsonOutput = binUtils.outputFormatter({ @@ -96,10 +94,10 @@ describe('bin/utils', () => { } expect(keys).toStrictEqual({ key1: 10, - key2: 8, + key2: 6, }); expect(tableOutput).toBe( - 'key1 \tkey2 \n"value1" \t"value2"\n"data1" \t"data2"\nN/A \tN/A\n', + 'key1 \tkey2 \nvalue1 \tvalue2\ndata1 \tdata2\nN/A \tN/A\n', ); }, ); @@ -112,7 +110,7 @@ describe('bin/utils', () => { type: 'dict', data: { key1: 'value1', key2: 'value2' }, }), - ).toBe('key1\t"value1"\nkey2\t"value2"\n'); + ).toBe('key1\tvalue1\nkey2\tvalue2\n'); expect( binUtils.outputFormatter({ type: 'dict', @@ -124,7 +122,7 @@ describe('bin/utils', () => { type: 'dict', data: { key1: null, key2: undefined }, }), - ).toBe('key1\t""\nkey2\t""\n'); + ).toBe('key1\t\nkey2\t\n'); // JSON expect( binUtils.outputFormatter({ @@ -148,11 +146,10 @@ describe('bin/utils', () => { }); // Construct the expected output - let expectedValue = binUtils.encodeQuotes(value); - expectedValue = binUtils.encodeNonPrintable(expectedValue); + let expectedValue = value; + expectedValue = binUtils.encodeEscapedWrapped(expectedValue); expectedValue = expectedValue.replace(/(?:\r\n|\n)$/, ''); expectedValue = expectedValue.replace(/(\r\n|\n)/g, '$1\t'); - expectedValue = `"${expectedValue}"`; const maxKeyLength = Math.max( ...Object.keys({ [key]: value }).map((k) => k.length), @@ -282,11 +279,16 @@ describe('bin/utils', () => { }, ); testUtils.testIf(testUtils.isTestPlatformEmpty)( - 'encoding non-printable strings works', + 'encodeEscaped should encode all escapable characters', () => { - expect(binUtils.encodeWrappedStrings('\n"\n"')).toBe('\n"\\n"'); - expect(binUtils.encodeWrappedStrings('"\\""')).toBe('"\\""'); - expect(binUtils.encodeWrappedStrings('"\\"\n\\""')).toBe('"\\"\\n\\""'); + fc.assert( + fc.property(stringWithNonPrintableCharsArb, (value) => { + expect(binUtils.decodeEscaped(binUtils.encodeEscaped(value))).toBe( + value, + ); + }), + { numRuns: 100 }, // Number of times to run the test + ); }, ); }); diff --git a/tests/vaults/vaults.test.ts b/tests/vaults/vaults.test.ts index 35578b1b..4f3d04b7 100644 --- a/tests/vaults/vaults.test.ts +++ b/tests/vaults/vaults.test.ts @@ -907,16 +907,18 @@ describe('CLI vaults', () => { cwd: dataDir, }); expect(result3.exitCode).toBe(0); - expect(result3.stdout).toMatch(/Vault1\\t\\t.*\\t\\tclone/); + expect(result3.stdout).toMatch(/Vault1 {4}.* {4}clone/); expect(result3.stdout).toContain( - `Vault1\\t\\t${vaultsUtils.encodeVaultId( + `Vault1${' '.repeat(4)}${vaultsUtils.encodeVaultId( vault1Id, - )}\\t\\tclone\nVault2\\t\\t${vaultsUtils.encodeVaultId( - vault2Id, - )}\\t\\tpull,clone\n`, + )}${' '.repeat(4)}clone\nVault2${' '.repeat( + 4, + )}${vaultsUtils.encodeVaultId(vault2Id)}${' '.repeat( + 4, + )}pull,clone\n`, ); expect(result3.stdout).not.toContain( - `Vault3\t\t${vaultsUtils.encodeVaultId(vault3Id)}`, + `Vault3${' '.repeat(4)}${vaultsUtils.encodeVaultId(vault3Id)}`, ); } finally { await remoteOnline?.stop();