From ee452a1e92b13b5799cef2b93cc90c1005fd1c2e Mon Sep 17 00:00:00 2001 From: pranay-v29 Date: Tue, 10 Nov 2026 16:00:21 +0530 Subject: [PATCH 1/2] [LOC-7659] fix(setup-local): keep every value of multi-value local-args flags validateLocalArgs split `local-args` on whitespace and parsed it with minimist, then deleted `_`. minimist keeps only the first token after a flag, so `--include-hosts hostA hostB` reached the binary as `--include-hosts hostA` and every other host was silently dropped (same for --exclude-hosts). Quoted values containing spaces were also broken into unbalanced fragments that swallowed the args appended after them. Tokenize `local-args` with a quote-aware splitter instead, keep each flag together with all the values that follow it, drop restricted flags along with their values, and re-quote values containing whitespace/double quotes so @actions/exec splits them back into the intended argv. minimist is no longer used and is removed; dist/ rebuilt. Co-Authored-By: Claude Opus 5.5 --- setup-local/dist/index.js | 350 ++++-------------- setup-local/package-lock.json | 1 - setup-local/package.json | 1 - setup-local/src/actionInput/inputValidator.js | 79 +++- .../test/actionInput/inputValidator.test.js | 51 ++- 5 files changed, 182 insertions(+), 300 deletions(-) diff --git a/setup-local/dist/index.js b/setup-local/dist/index.js index dcd45c09..4fa451c8 100644 --- a/setup-local/dist/index.js +++ b/setup-local/dist/index.js @@ -66817,277 +66817,6 @@ var union = baseRest(function(arrays) { module.exports = union; -/***/ }), - -/***/ 80994: -/***/ ((module) => { - -"use strict"; - - -function hasKey(obj, keys) { - var o = obj; - keys.slice(0, -1).forEach(function (key) { - o = o[key] || {}; - }); - - var key = keys[keys.length - 1]; - return key in o; -} - -function isNumber(x) { - if (typeof x === 'number') { return true; } - if ((/^0x[0-9a-f]+$/i).test(x)) { return true; } - return (/^[-+]?(?:\d+(?:\.\d*)?|\.\d+)(e[-+]?\d+)?$/).test(x); -} - -function isConstructorOrProto(obj, key) { - return (key === 'constructor' && typeof obj[key] === 'function') || key === '__proto__'; -} - -module.exports = function (args, opts) { - if (!opts) { opts = {}; } - - var flags = { - bools: {}, - strings: {}, - unknownFn: null, - }; - - if (typeof opts.unknown === 'function') { - flags.unknownFn = opts.unknown; - } - - if (typeof opts.boolean === 'boolean' && opts.boolean) { - flags.allBools = true; - } else { - [].concat(opts.boolean).filter(Boolean).forEach(function (key) { - flags.bools[key] = true; - }); - } - - var aliases = {}; - - function aliasIsBoolean(key) { - return aliases[key].some(function (x) { - return flags.bools[x]; - }); - } - - Object.keys(opts.alias || {}).forEach(function (key) { - aliases[key] = [].concat(opts.alias[key]); - aliases[key].forEach(function (x) { - aliases[x] = [key].concat(aliases[key].filter(function (y) { - return x !== y; - })); - }); - }); - - [].concat(opts.string).filter(Boolean).forEach(function (key) { - flags.strings[key] = true; - if (aliases[key]) { - [].concat(aliases[key]).forEach(function (k) { - flags.strings[k] = true; - }); - } - }); - - var defaults = opts.default || {}; - - var argv = { _: [] }; - - function argDefined(key, arg) { - return (flags.allBools && (/^--[^=]+$/).test(arg)) - || flags.strings[key] - || flags.bools[key] - || aliases[key]; - } - - function setKey(obj, keys, value) { - var o = obj; - for (var i = 0; i < keys.length - 1; i++) { - var key = keys[i]; - if (isConstructorOrProto(o, key)) { return; } - if (o[key] === undefined) { o[key] = {}; } - if ( - o[key] === Object.prototype - || o[key] === Number.prototype - || o[key] === String.prototype - ) { - o[key] = {}; - } - if (o[key] === Array.prototype) { o[key] = []; } - o = o[key]; - } - - var lastKey = keys[keys.length - 1]; - if (isConstructorOrProto(o, lastKey)) { return; } - if ( - o === Object.prototype - || o === Number.prototype - || o === String.prototype - ) { - o = {}; - } - if (o === Array.prototype) { o = []; } - if (o[lastKey] === undefined || flags.bools[lastKey] || typeof o[lastKey] === 'boolean') { - o[lastKey] = value; - } else if (Array.isArray(o[lastKey])) { - o[lastKey].push(value); - } else { - o[lastKey] = [o[lastKey], value]; - } - } - - function setArg(key, val, arg) { - if (arg && flags.unknownFn && !argDefined(key, arg)) { - if (flags.unknownFn(arg) === false) { return; } - } - - var value = !flags.strings[key] && isNumber(val) - ? Number(val) - : val; - setKey(argv, key.split('.'), value); - - (aliases[key] || []).forEach(function (x) { - setKey(argv, x.split('.'), value); - }); - } - - Object.keys(flags.bools).forEach(function (key) { - setArg(key, defaults[key] === undefined ? false : defaults[key]); - }); - - var notFlags = []; - - if (args.indexOf('--') !== -1) { - notFlags = args.slice(args.indexOf('--') + 1); - args = args.slice(0, args.indexOf('--')); - } - - for (var i = 0; i < args.length; i++) { - var arg = args[i]; - var key; - var next; - - if ((/^--.+=/).test(arg)) { - // Using [\s\S] instead of . because js doesn't support the - // 'dotall' regex modifier. See: - // http://stackoverflow.com/a/1068308/13216 - var m = arg.match(/^--([^=]+)=([\s\S]*)$/); - key = m[1]; - var value = m[2]; - if (flags.bools[key]) { - value = value !== 'false'; - } - setArg(key, value, arg); - } else if ((/^--no-.+/).test(arg)) { - key = arg.match(/^--no-(.+)/)[1]; - setArg(key, false, arg); - } else if ((/^--.+/).test(arg)) { - key = arg.match(/^--(.+)/)[1]; - next = args[i + 1]; - if ( - next !== undefined - && !(/^(-|--)[^-]/).test(next) - && !flags.bools[key] - && !flags.allBools - && (aliases[key] ? !aliasIsBoolean(key) : true) - ) { - setArg(key, next, arg); - i += 1; - } else if ((/^(true|false)$/).test(next)) { - setArg(key, next === 'true', arg); - i += 1; - } else { - setArg(key, flags.strings[key] ? '' : true, arg); - } - } else if ((/^-[^-]+/).test(arg)) { - var letters = arg.slice(1, -1).split(''); - - var broken = false; - for (var j = 0; j < letters.length; j++) { - next = arg.slice(j + 2); - - if (next === '-') { - setArg(letters[j], next, arg); - continue; - } - - if ((/[A-Za-z]/).test(letters[j]) && next[0] === '=') { - setArg(letters[j], next.slice(1), arg); - broken = true; - break; - } - - if ( - (/[A-Za-z]/).test(letters[j]) - && (/-?\d+(\.\d*)?(e-?\d+)?$/).test(next) - ) { - setArg(letters[j], next, arg); - broken = true; - break; - } - - if (letters[j + 1] && letters[j + 1].match(/\W/)) { - setArg(letters[j], arg.slice(j + 2), arg); - broken = true; - break; - } else { - setArg(letters[j], flags.strings[letters[j]] ? '' : true, arg); - } - } - - key = arg.slice(-1)[0]; - if (!broken && key !== '-') { - if ( - args[i + 1] - && !(/^(-|--)[^-]/).test(args[i + 1]) - && !flags.bools[key] - && (aliases[key] ? !aliasIsBoolean(key) : true) - ) { - setArg(key, args[i + 1], arg); - i += 1; - } else if (args[i + 1] && (/^(true|false)$/).test(args[i + 1])) { - setArg(key, args[i + 1] === 'true', arg); - i += 1; - } else { - setArg(key, flags.strings[key] ? '' : true, arg); - } - } - } else { - if (!flags.unknownFn || flags.unknownFn(arg) !== false) { - argv._.push(flags.strings._ || !isNumber(arg) ? arg : Number(arg)); - } - if (opts.stopEarly) { - argv._.push.apply(argv._, args.slice(i + 1)); - break; - } - } - } - - Object.keys(defaults).forEach(function (k) { - if (!hasKey(argv, k.split('.'))) { - setKey(argv, k.split('.'), defaults[k]); - - (aliases[k] || []).forEach(function (x) { - setKey(argv, x.split('.'), defaults[k]); - }); - } - }); - - if (opts['--']) { - argv['--'] = notFlags.slice(); - } else { - notFlags.forEach(function (k) { - argv._.push(k); - }); - } - - return argv; -}; - - /***/ }), /***/ 4469: @@ -105845,7 +105574,6 @@ module.exports = ActionInput; const core = __nccwpck_require__(37484); const { v4: uuidv4 } = __nccwpck_require__(12048); -const parseArgs = __nccwpck_require__(80994); const constants = __nccwpck_require__(69350); const { @@ -105929,28 +105657,82 @@ class InputValidator { return inputLocalIdentifier.split(/\s+/).join('-'); } + /** + * Splits a string into tokens on whitespace, keeping single/double quoted + * sections together (the quotes themselves are removed). + * @param {String} argString Raw args string + * @returns {Array} Tokens + */ + static _tokenizeArgs(argString) { + const tokens = []; + let current = ''; + let inToken = false; + let quoteChar = null; + + for (const char of argString) { + if (quoteChar) { + if (char === quoteChar) { + quoteChar = null; + } else { + current += char; + } + } else if (char === '"' || char === "'") { + quoteChar = char; + inToken = true; + } else if (/\s/.test(char)) { + if (inToken) { + tokens.push(current); + current = ''; + inToken = false; + } + } else { + current += char; + inToken = true; + } + } + if (inToken) tokens.push(current); + + return tokens; + } + + /** + * Quotes a token for the command line consumed by @actions/exec, which + * understands double quotes and backslash-escaped double quotes (other + * backslashes are passed through untouched). + * @param {String} token Single arg token + * @returns {String} Token safe to place in the command string + */ + static _quoteArg(token) { + if (!/[\s"]/.test(token)) return token; + return `"${token.replace(/"/g, '\\"')}"`; + } + /** * Validates the local-args input. Removes any args which might conflict with * the input args taken from the action input for the Local Binary. + * Every value following a flag is preserved, so flags taking multiple values + * (e.g. --include-hosts host1 host2) are passed to the binary intact. * @param {String} inputLocalArgs Action input for 'local-args' * @returns {String} Parsed args */ static validateLocalArgs(inputLocalArgs) { - const parsedArgs = parseArgs(inputLocalArgs.split(/\s+/)); + if (!inputLocalArgs) return ''; - delete parsedArgs._; - RESTRICTED_LOCAL_ARGS.forEach((arg) => { - delete parsedArgs[arg]; - }); + const tokens = InputValidator._tokenizeArgs(inputLocalArgs); + const isFlag = (token) => /^-{1,2}[^-\d]/.test(token); - let parsedArgsString = ''; - for (const [key, value] of Object.entries(parsedArgs)) { - const argKey = key.length === 1 ? `-${key}` : `--${key}`; - const argValue = value === true ? '' : value; - parsedArgsString += `${argKey} ${argValue} `; - } + const keptTokens = []; + // Tokens before the first flag have no flag to belong to and are dropped. + let keepCurrentGroup = false; + tokens.forEach((token) => { + if (isFlag(token)) { + const flagName = token.replace(/^-{1,2}/, '').split('=')[0]; + keepCurrentGroup = !RESTRICTED_LOCAL_ARGS.includes(flagName); + } + if (keepCurrentGroup) keptTokens.push(InputValidator._quoteArg(token)); + }); - return parsedArgsString; + return keptTokens.join(' '); } } diff --git a/setup-local/package-lock.json b/setup-local/package-lock.json index afe9ac51..ee785f30 100644 --- a/setup-local/package-lock.json +++ b/setup-local/package-lock.json @@ -15,7 +15,6 @@ "@actions/github": "^6.0.0", "@actions/io": "^1.0.2", "@actions/tool-cache": "^1.6.0", - "minimist": "^1.2.6", "uuid": "^8.3.0" }, "devDependencies": { diff --git a/setup-local/package.json b/setup-local/package.json index bdcf24e4..5f6c6c39 100644 --- a/setup-local/package.json +++ b/setup-local/package.json @@ -37,7 +37,6 @@ "@actions/github": "^6.0.0", "@actions/io": "^1.0.2", "@actions/tool-cache": "^1.6.0", - "minimist": "^1.2.6", "uuid": "^8.3.0" }, "devDependencies": { diff --git a/setup-local/src/actionInput/inputValidator.js b/setup-local/src/actionInput/inputValidator.js index ae278607..3854132d 100644 --- a/setup-local/src/actionInput/inputValidator.js +++ b/setup-local/src/actionInput/inputValidator.js @@ -1,6 +1,5 @@ const core = require('@actions/core'); const { v4: uuidv4 } = require('uuid'); -const parseArgs = require('minimist'); const constants = require('../../config/constants'); const { @@ -84,28 +83,82 @@ class InputValidator { return inputLocalIdentifier.split(/\s+/).join('-'); } + /** + * Splits a string into tokens on whitespace, keeping single/double quoted + * sections together (the quotes themselves are removed). + * @param {String} argString Raw args string + * @returns {Array} Tokens + */ + static _tokenizeArgs(argString) { + const tokens = []; + let current = ''; + let inToken = false; + let quoteChar = null; + + for (const char of argString) { + if (quoteChar) { + if (char === quoteChar) { + quoteChar = null; + } else { + current += char; + } + } else if (char === '"' || char === "'") { + quoteChar = char; + inToken = true; + } else if (/\s/.test(char)) { + if (inToken) { + tokens.push(current); + current = ''; + inToken = false; + } + } else { + current += char; + inToken = true; + } + } + if (inToken) tokens.push(current); + + return tokens; + } + + /** + * Quotes a token for the command line consumed by @actions/exec, which + * understands double quotes and backslash-escaped double quotes (other + * backslashes are passed through untouched). + * @param {String} token Single arg token + * @returns {String} Token safe to place in the command string + */ + static _quoteArg(token) { + if (!/[\s"]/.test(token)) return token; + return `"${token.replace(/"/g, '\\"')}"`; + } + /** * Validates the local-args input. Removes any args which might conflict with * the input args taken from the action input for the Local Binary. + * Every value following a flag is preserved, so flags taking multiple values + * (e.g. --include-hosts host1 host2) are passed to the binary intact. * @param {String} inputLocalArgs Action input for 'local-args' * @returns {String} Parsed args */ static validateLocalArgs(inputLocalArgs) { - const parsedArgs = parseArgs(inputLocalArgs.split(/\s+/)); + if (!inputLocalArgs) return ''; - delete parsedArgs._; - RESTRICTED_LOCAL_ARGS.forEach((arg) => { - delete parsedArgs[arg]; - }); + const tokens = InputValidator._tokenizeArgs(inputLocalArgs); + const isFlag = (token) => /^-{1,2}[^-\d]/.test(token); - let parsedArgsString = ''; - for (const [key, value] of Object.entries(parsedArgs)) { - const argKey = key.length === 1 ? `-${key}` : `--${key}`; - const argValue = value === true ? '' : value; - parsedArgsString += `${argKey} ${argValue} `; - } + const keptTokens = []; + // Tokens before the first flag have no flag to belong to and are dropped. + let keepCurrentGroup = false; + tokens.forEach((token) => { + if (isFlag(token)) { + const flagName = token.replace(/^-{1,2}/, '').split('=')[0]; + keepCurrentGroup = !RESTRICTED_LOCAL_ARGS.includes(flagName); + } + if (keepCurrentGroup) keptTokens.push(InputValidator._quoteArg(token)); + }); - return parsedArgsString; + return keptTokens.join(' '); } } diff --git a/setup-local/test/actionInput/inputValidator.test.js b/setup-local/test/actionInput/inputValidator.test.js index db883dc1..ef110f8b 100644 --- a/setup-local/test/actionInput/inputValidator.test.js +++ b/setup-local/test/actionInput/inputValidator.test.js @@ -1,6 +1,7 @@ const core = require('@actions/core'); const { expect } = require('chai'); const sinon = require('sinon'); +const { argStringToArray } = require('@actions/exec/lib/toolrunner'); const constants = require('../../config/constants'); const InputValidator = require('../../src/actionInput/inputValidator'); @@ -79,9 +80,57 @@ describe('InputValidator class to validate individual fields of the action input context('Validates local args', () => { it('Removes the restricted/not-alowed args from the local-args input and returns the string', () => { const inputLocalArgs = '--key someKey --proxy-host hostname --someOtherKey someValue -z --daemon start --ci-plugin someName -k anotherKey --only-automate --log-file some/path/ --verbose level --local-identifier someIdentifier'; - const expectedLocalArgs = '--proxy-host hostname --someOtherKey someValue -z '; + const expectedLocalArgs = '--proxy-host hostname --someOtherKey someValue -z'; expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); }); + + ['', null, undefined].forEach((value) => { + it(`Returns empty string if the input is :${JSON.stringify(value)}`, () => { + expect(InputValidator.validateLocalArgs(value)).to.eq(''); + }); + }); + + it('Keeps every value of a flag that takes multiple values (quoted)', () => { + const inputLocalArgs = '--include-hosts "hostA.example.com:443" "hostB.example.com:443" --force-local'; + const expectedLocalArgs = '--include-hosts hostA.example.com:443 hostB.example.com:443 --force-local'; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + }); + + it('Keeps every value of a flag that takes multiple values (unquoted, multi-line)', () => { + const inputLocalArgs = '--include-hosts hostA.example.com\n hostB.example.com\n--exclude-hosts hostC hostD'; + const expectedLocalArgs = '--include-hosts hostA.example.com hostB.example.com --exclude-hosts hostC hostD'; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + }); + + it('Removes the values of restricted args even when they have multiple values', () => { + const inputLocalArgs = '--include-hosts hostA hostB --verbose 3 extra --local-identifier=abc --key=secret --proxy-port 8080'; + const expectedLocalArgs = '--include-hosts hostA hostB --proxy-port 8080'; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + }); + + it('Drops tokens that appear before the first flag', () => { + expect(InputValidator.validateLocalArgs('stray value --force-local')).to.eq('--force-local'); + }); + + it('Keeps negative numbers as values rather than treating them as flags', () => { + expect(InputValidator.validateLocalArgs('--some-flag -1 --other')).to.eq('--some-flag -1 --other'); + }); + + it('Re-quotes values containing whitespace or double quotes', () => { + const inputLocalArgs = `--folder "/path with spaces/dir" --name 'say "hi"'`; + const expectedLocalArgs = '--folder "/path with spaces/dir" --name "say \\"hi\\""'; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + }); + + it('Produces a string that @actions/exec splits back into the intended argv', () => { + const inputLocalArgs = `--include-hosts "hostA:443" "hostB:443" --folder "C:\\my dir\\site" --name 'say "hi"'`; + const parsed = argStringToArray(InputValidator.validateLocalArgs(inputLocalArgs)); + expect(parsed).to.deep.eq([ + '--include-hosts', 'hostA:443', 'hostB:443', + '--folder', 'C:\\my dir\\site', + '--name', 'say "hi"', + ]); + }); }); }); }); From 9e02c71af4aec393afc3198955d71335ff427d52 Mon Sep 17 00:00:00 2001 From: pranay-v29 Date: Tue, 10 Nov 2026 18:43:06 +0530 Subject: [PATCH 2/2] [LOC-7659] fix(setup-local): pass binary args as an argv array Address CodeQL js/incomplete-sanitization on _quoteArg: re-quoting tokens into a command string for @actions/exec's argStringToArray cannot represent every value (e.g. a backslash right before a closing quote, as in a Windows path ending in '\'). Drop the string round-trip entirely: validateLocalArgs returns an array of tokens, _generateArgsForBinary builds an argv array, and _triggerBinary calls exec.exec(binary, args). Values now reach the binary verbatim with no escaping, which also fixes a --log-file path containing spaces. Co-Authored-By: Claude Opus 5.5 --- setup-local/dist/index.js | 40 ++++++---------- setup-local/src/actionInput/index.js | 2 +- setup-local/src/actionInput/inputValidator.js | 20 ++------ setup-local/src/binaryControl.js | 18 +++---- .../test/actionInput/inputValidator.test.js | 45 ++++++++--------- setup-local/test/binaryControl.test.js | 48 ++++++++++--------- 6 files changed, 75 insertions(+), 98 deletions(-) diff --git a/setup-local/dist/index.js b/setup-local/dist/index.js index 4fa451c8..b4e1f771 100644 --- a/setup-local/dist/index.js +++ b/setup-local/dist/index.js @@ -105548,7 +105548,7 @@ class ActionInput { * @returns {{ * accessKey: String, * localTesting: String, - * localArgs: String, + * localArgs: Array, * localIdentifier: String, * localLoggingLevel: Number * }} @@ -105695,28 +105695,16 @@ class InputValidator { return tokens; } - /** - * Quotes a token for the command line consumed by @actions/exec, which - * understands double quotes and backslash-escaped double quotes (other - * backslashes are passed through untouched). - * @param {String} token Single arg token - * @returns {String} Token safe to place in the command string - */ - static _quoteArg(token) { - if (!/[\s"]/.test(token)) return token; - return `"${token.replace(/"/g, '\\"')}"`; - } - /** * Validates the local-args input. Removes any args which might conflict with * the input args taken from the action input for the Local Binary. * Every value following a flag is preserved, so flags taking multiple values * (e.g. --include-hosts host1 host2) are passed to the binary intact. * @param {String} inputLocalArgs Action input for 'local-args' - * @returns {String} Parsed args + * @returns {Array} Parsed args, one element per argv entry */ static validateLocalArgs(inputLocalArgs) { - if (!inputLocalArgs) return ''; + if (!inputLocalArgs) return []; const tokens = InputValidator._tokenizeArgs(inputLocalArgs); const isFlag = (token) => /^-{1,2}[^-\d]/.test(token); @@ -105729,10 +105717,10 @@ class InputValidator { const flagName = token.replace(/^-{1,2}/, '').split('=')[0]; keepCurrentGroup = !RESTRICTED_LOCAL_ARGS.includes(flagName); } - if (keepCurrentGroup) keptTokens.push(InputValidator._quoteArg(token)); + if (keepCurrentGroup) keptTokens.push(token); }); - return keptTokens.join(' '); + return keptTokens; } } @@ -105879,20 +105867,22 @@ class BinaryControl { localTesting: binaryAction, } = this.stateForBinary; - let argsString = `--key ${key} --only-automate --ci-plugin GitHubAction `; + // Built as an argv array (not a command string) so that values containing + // spaces/quotes/backslashes reach the binary verbatim, without re-parsing. + const args = ['--key', key, '--only-automate', '--ci-plugin', 'GitHubAction']; switch (binaryAction) { case LOCAL_TESTING.START: { - if (localArgs) argsString += `${localArgs} `; - if (localIdentifier) argsString += `--local-identifier ${localIdentifier} `; + if (localArgs && localArgs.length) args.push(...localArgs); + if (localIdentifier) args.push('--local-identifier', localIdentifier); if (verbose) { this._generateLogFileMetadata(); - argsString += `--verbose ${verbose} --log-file ${this.logFilePath} `; + args.push('--verbose', `${verbose}`, '--log-file', this.logFilePath); } break; } case LOCAL_TESTING.STOP: { - if (localIdentifier) argsString += `--local-identifier ${localIdentifier} `; + if (localIdentifier) args.push('--local-identifier', localIdentifier); break; } default: { @@ -105900,7 +105890,7 @@ class BinaryControl { } } - this.binaryArgs = argsString; + this.binaryArgs = args; } /** @@ -105911,8 +105901,8 @@ class BinaryControl { let triggerOutput = ''; let triggerError = ''; await exec.exec( - `${LOCAL_BINARY_NAME} ${this.binaryArgs} --daemon ${operation}`, - [], + LOCAL_BINARY_NAME, + [...this.binaryArgs, '--daemon', operation], { listeners: { stdout: (data) => { diff --git a/setup-local/src/actionInput/index.js b/setup-local/src/actionInput/index.js index 87b2fcbb..6446d8ce 100644 --- a/setup-local/src/actionInput/index.js +++ b/setup-local/src/actionInput/index.js @@ -77,7 +77,7 @@ class ActionInput { * @returns {{ * accessKey: String, * localTesting: String, - * localArgs: String, + * localArgs: Array, * localIdentifier: String, * localLoggingLevel: Number * }} diff --git a/setup-local/src/actionInput/inputValidator.js b/setup-local/src/actionInput/inputValidator.js index 3854132d..f348c11f 100644 --- a/setup-local/src/actionInput/inputValidator.js +++ b/setup-local/src/actionInput/inputValidator.js @@ -121,28 +121,16 @@ class InputValidator { return tokens; } - /** - * Quotes a token for the command line consumed by @actions/exec, which - * understands double quotes and backslash-escaped double quotes (other - * backslashes are passed through untouched). - * @param {String} token Single arg token - * @returns {String} Token safe to place in the command string - */ - static _quoteArg(token) { - if (!/[\s"]/.test(token)) return token; - return `"${token.replace(/"/g, '\\"')}"`; - } - /** * Validates the local-args input. Removes any args which might conflict with * the input args taken from the action input for the Local Binary. * Every value following a flag is preserved, so flags taking multiple values * (e.g. --include-hosts host1 host2) are passed to the binary intact. * @param {String} inputLocalArgs Action input for 'local-args' - * @returns {String} Parsed args + * @returns {Array} Parsed args, one element per argv entry */ static validateLocalArgs(inputLocalArgs) { - if (!inputLocalArgs) return ''; + if (!inputLocalArgs) return []; const tokens = InputValidator._tokenizeArgs(inputLocalArgs); const isFlag = (token) => /^-{1,2}[^-\d]/.test(token); @@ -155,10 +143,10 @@ class InputValidator { const flagName = token.replace(/^-{1,2}/, '').split('=')[0]; keepCurrentGroup = !RESTRICTED_LOCAL_ARGS.includes(flagName); } - if (keepCurrentGroup) keptTokens.push(InputValidator._quoteArg(token)); + if (keepCurrentGroup) keptTokens.push(token); }); - return keptTokens.join(' '); + return keptTokens; } } diff --git a/setup-local/src/binaryControl.js b/setup-local/src/binaryControl.js index 1be93293..49ee2b57 100644 --- a/setup-local/src/binaryControl.js +++ b/setup-local/src/binaryControl.js @@ -98,20 +98,22 @@ class BinaryControl { localTesting: binaryAction, } = this.stateForBinary; - let argsString = `--key ${key} --only-automate --ci-plugin GitHubAction `; + // Built as an argv array (not a command string) so that values containing + // spaces/quotes/backslashes reach the binary verbatim, without re-parsing. + const args = ['--key', key, '--only-automate', '--ci-plugin', 'GitHubAction']; switch (binaryAction) { case LOCAL_TESTING.START: { - if (localArgs) argsString += `${localArgs} `; - if (localIdentifier) argsString += `--local-identifier ${localIdentifier} `; + if (localArgs && localArgs.length) args.push(...localArgs); + if (localIdentifier) args.push('--local-identifier', localIdentifier); if (verbose) { this._generateLogFileMetadata(); - argsString += `--verbose ${verbose} --log-file ${this.logFilePath} `; + args.push('--verbose', `${verbose}`, '--log-file', this.logFilePath); } break; } case LOCAL_TESTING.STOP: { - if (localIdentifier) argsString += `--local-identifier ${localIdentifier} `; + if (localIdentifier) args.push('--local-identifier', localIdentifier); break; } default: { @@ -119,7 +121,7 @@ class BinaryControl { } } - this.binaryArgs = argsString; + this.binaryArgs = args; } /** @@ -130,8 +132,8 @@ class BinaryControl { let triggerOutput = ''; let triggerError = ''; await exec.exec( - `${LOCAL_BINARY_NAME} ${this.binaryArgs} --daemon ${operation}`, - [], + LOCAL_BINARY_NAME, + [...this.binaryArgs, '--daemon', operation], { listeners: { stdout: (data) => { diff --git a/setup-local/test/actionInput/inputValidator.test.js b/setup-local/test/actionInput/inputValidator.test.js index ef110f8b..50e5ec7e 100644 --- a/setup-local/test/actionInput/inputValidator.test.js +++ b/setup-local/test/actionInput/inputValidator.test.js @@ -1,7 +1,6 @@ const core = require('@actions/core'); const { expect } = require('chai'); const sinon = require('sinon'); -const { argStringToArray } = require('@actions/exec/lib/toolrunner'); const constants = require('../../config/constants'); const InputValidator = require('../../src/actionInput/inputValidator'); @@ -80,55 +79,49 @@ describe('InputValidator class to validate individual fields of the action input context('Validates local args', () => { it('Removes the restricted/not-alowed args from the local-args input and returns the string', () => { const inputLocalArgs = '--key someKey --proxy-host hostname --someOtherKey someValue -z --daemon start --ci-plugin someName -k anotherKey --only-automate --log-file some/path/ --verbose level --local-identifier someIdentifier'; - const expectedLocalArgs = '--proxy-host hostname --someOtherKey someValue -z'; - expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + const expectedLocalArgs = ['--proxy-host', 'hostname', '--someOtherKey', 'someValue', '-z']; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.deep.eq(expectedLocalArgs); }); ['', null, undefined].forEach((value) => { - it(`Returns empty string if the input is :${JSON.stringify(value)}`, () => { - expect(InputValidator.validateLocalArgs(value)).to.eq(''); + it(`Returns an empty array if the input is :${JSON.stringify(value)}`, () => { + expect(InputValidator.validateLocalArgs(value)).to.deep.eq([]); }); }); it('Keeps every value of a flag that takes multiple values (quoted)', () => { const inputLocalArgs = '--include-hosts "hostA.example.com:443" "hostB.example.com:443" --force-local'; - const expectedLocalArgs = '--include-hosts hostA.example.com:443 hostB.example.com:443 --force-local'; - expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.deep.eq([ + '--include-hosts', 'hostA.example.com:443', 'hostB.example.com:443', '--force-local', + ]); }); it('Keeps every value of a flag that takes multiple values (unquoted, multi-line)', () => { const inputLocalArgs = '--include-hosts hostA.example.com\n hostB.example.com\n--exclude-hosts hostC hostD'; - const expectedLocalArgs = '--include-hosts hostA.example.com hostB.example.com --exclude-hosts hostC hostD'; - expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.deep.eq([ + '--include-hosts', 'hostA.example.com', 'hostB.example.com', '--exclude-hosts', 'hostC', 'hostD', + ]); }); it('Removes the values of restricted args even when they have multiple values', () => { const inputLocalArgs = '--include-hosts hostA hostB --verbose 3 extra --local-identifier=abc --key=secret --proxy-port 8080'; - const expectedLocalArgs = '--include-hosts hostA hostB --proxy-port 8080'; - expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.deep.eq([ + '--include-hosts', 'hostA', 'hostB', '--proxy-port', '8080', + ]); }); it('Drops tokens that appear before the first flag', () => { - expect(InputValidator.validateLocalArgs('stray value --force-local')).to.eq('--force-local'); + expect(InputValidator.validateLocalArgs('stray value --force-local')).to.deep.eq(['--force-local']); }); it('Keeps negative numbers as values rather than treating them as flags', () => { - expect(InputValidator.validateLocalArgs('--some-flag -1 --other')).to.eq('--some-flag -1 --other'); - }); - - it('Re-quotes values containing whitespace or double quotes', () => { - const inputLocalArgs = `--folder "/path with spaces/dir" --name 'say "hi"'`; - const expectedLocalArgs = '--folder "/path with spaces/dir" --name "say \\"hi\\""'; - expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.eq(expectedLocalArgs); + expect(InputValidator.validateLocalArgs('--some-flag -1 --other')).to.deep.eq(['--some-flag', '-1', '--other']); }); - it('Produces a string that @actions/exec splits back into the intended argv', () => { - const inputLocalArgs = `--include-hosts "hostA:443" "hostB:443" --folder "C:\\my dir\\site" --name 'say "hi"'`; - const parsed = argStringToArray(InputValidator.validateLocalArgs(inputLocalArgs)); - expect(parsed).to.deep.eq([ - '--include-hosts', 'hostA:443', 'hostB:443', - '--folder', 'C:\\my dir\\site', - '--name', 'say "hi"', + it('Keeps quoted values with spaces, quotes and backslashes as single verbatim args', () => { + const inputLocalArgs = `--folder "C:\\my dir\\" --name 'say "hi"' --other "it's"`; + expect(InputValidator.validateLocalArgs(inputLocalArgs)).to.deep.eq([ + '--folder', 'C:\\my dir\\', '--name', 'say "hi"', '--other', "it's", ]); }); }); diff --git a/setup-local/test/binaryControl.test.js b/setup-local/test/binaryControl.test.js index 864b8eea..74c564c0 100644 --- a/setup-local/test/binaryControl.test.js +++ b/setup-local/test/binaryControl.test.js @@ -199,7 +199,7 @@ describe('Binary Control Operations', () => { it('with localArgs, localIdentifier, localLoggingLevel', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '--arg1 val1 --arg2 val2', + localArgs: ['--arg1', 'val1', '--arg2', 'val2'], localIdentifier: 'someIdentifier', localLoggingLevel: 1, localTesting: 'start', @@ -214,55 +214,59 @@ describe('Binary Control Operations', () => { os.platform(), 'BrowserStackLocal_someJobName_now.log', ); - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction --arg1 val1 --arg2 val2 --local-identifier someIdentifier --verbose 1 --log-file ${expectedLogFilePath} `; + const expectedFinalArgs = [ + '--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction', + '--arg1', 'val1', '--arg2', 'val2', + '--local-identifier', 'someIdentifier', '--verbose', '1', '--log-file', expectedLogFilePath, + ]; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); it('with localArgs, localIdentifier', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '--arg1 val1 --arg2 val2', + localArgs: ['--arg1', 'val1', '--arg2', 'val2'], localIdentifier: 'someIdentifier', localLoggingLevel: 0, localTesting: 'start', }; - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction --arg1 val1 --arg2 val2 --local-identifier someIdentifier `; + const expectedFinalArgs = ['--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction', '--arg1', 'val1', '--arg2', 'val2', '--local-identifier', 'someIdentifier']; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); it('with localArgs', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '--arg1 val1 --arg2 val2', + localArgs: ['--arg1', 'val1', '--arg2', 'val2'], localIdentifier: '', localLoggingLevel: 0, localTesting: 'start', }; - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction --arg1 val1 --arg2 val2 `; + const expectedFinalArgs = ['--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction', '--arg1', 'val1', '--arg2', 'val2']; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); it('with the default args', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '', + localArgs: [], localIdentifier: '', localLoggingLevel: 0, localTesting: 'start', }; - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction `; + const expectedFinalArgs = ['--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction']; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); }); @@ -270,31 +274,31 @@ describe('Binary Control Operations', () => { it('with localIdentifier', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '', + localArgs: [], localIdentifier: 'someIdentifier', localLoggingLevel: 0, localTesting: 'stop', }; - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction --local-identifier someIdentifier `; + const expectedFinalArgs = ['--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction', '--local-identifier', 'someIdentifier']; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); it('without localIdentifier', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '', + localArgs: [], localIdentifier: '', localLoggingLevel: 0, localTesting: 'stop', }; - const expectedFinalArgs = `--key someKey --only-automate --ci-plugin GitHubAction `; + const expectedFinalArgs = ['--key', 'someKey', '--only-automate', '--ci-plugin', 'GitHubAction']; const binaryControl = new BinaryControl(stateForBinary); binaryControl._generateArgsForBinary(); - expect(binaryControl.binaryArgs).to.eq(expectedFinalArgs); + expect(binaryControl.binaryArgs).to.deep.eq(expectedFinalArgs); }); }); @@ -302,7 +306,7 @@ describe('Binary Control Operations', () => { it('Throws error in case its not start/stop', () => { const stateForBinary = { accessKey: 'someKey', - localArgs: '', + localArgs: [], localIdentifier: 'someIdentifier', localLoggingLevel: 0, localTesting: 'someOperation', @@ -322,7 +326,7 @@ describe('Binary Control Operations', () => { beforeEach(() => { sinon.stub(exec, 'exec'); binaryControl = new BinaryControl(); - binaryControl.binaryArgs = 'someArgs '; + binaryControl.binaryArgs = ['someArgs']; }); afterEach(() => { @@ -331,7 +335,7 @@ describe('Binary Control Operations', () => { it('Start Operation', async () => { const response = await binaryControl._triggerBinary(LOCAL_TESTING.START); - sinon.assert.calledWith(exec.exec, `${LOCAL_BINARY_NAME} someArgs --daemon start`); + sinon.assert.calledWith(exec.exec, LOCAL_BINARY_NAME, ['someArgs', '--daemon', 'start']); expect(response).to.eql({ output: '', error: '', @@ -340,7 +344,7 @@ describe('Binary Control Operations', () => { it('Stop Operation', async () => { const response = await binaryControl._triggerBinary(LOCAL_TESTING.STOP); - sinon.assert.calledWith(exec.exec, `${LOCAL_BINARY_NAME} someArgs --daemon stop`); + sinon.assert.calledWith(exec.exec, LOCAL_BINARY_NAME, ['someArgs', '--daemon', 'stop']); expect(response).to.eql({ output: '', error: '',