From dc9bdec25f7176a8ab498854ab3931f914b7ec0c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Feb 2022 22:00:26 +0100 Subject: [PATCH] Fix validation of tmake-server-xxx options Use appropriate regexes for each parameter instead of using the same, wrong, regex for all of them and fix the direction of test. Add unit tests to verify that the validation works as expected. --- action.yml | 4 ++-- lib/index.js | 27 +++++++++++++++++------- src/helpers.js | 7 +++--- src/index.js | 20 +++++++++++++----- src/index.test.js | 54 ++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 93 insertions(+), 19 deletions(-) diff --git a/action.yml b/action.yml index e641e7e0..fd33158e 100644 --- a/action.yml +++ b/action.yml @@ -20,11 +20,11 @@ inputs: required: false default: 'false' tmate-server-host: - description: 'The hostname for your tmate server' + description: 'The hostname for your tmate server (e.g. ssh.example.org)' required: false default: '' tmate-server-port: - description: 'The port for your tmate server' + description: 'The port for your tmate server (e.g. 2222)' required: false default: '' tmate-server-rsa-fingerprint: diff --git a/lib/index.js b/lib/index.js index e670611d..5bf08497 100644 --- a/lib/index.js +++ b/lib/index.js @@ -10282,11 +10282,12 @@ const execShellCommand = (cmd) => { /** * @param {string} key - * @return {string} + * @param {regex} re regex to use for validation + * @return {string}, {undefined} or throws an error if input doesn't match regex */ -const getValidatedInput = (key) => { +const getValidatedInput = (key, re) => { const value = core.getInput(key); - if (/^[-.+A-Za-z0-9]*$/.test(value)) { + if (value !== undefined && !re.test(value)) { throw new Error(`Invalid value for '${key}': '${value}'`); } return value; @@ -10406,11 +10407,21 @@ async function run() { await execShellCommand(`echo 'set +e' >/tmp/tmate.bashrc`); let setDefaultCommand = `set-option -g default-command "bash --rcfile /tmp/tmate.bashrc" \\;`; - if (core.getInput("tmate-server-host") !== "") { - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-host "${getValidatedInput("tmate-server-host")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-port "${getValidatedInput("tmate-server-port")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-rsa-fingerprint "${getValidatedInput("tmate-server-rsa-fingerprint")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-ed25519-fingerprint "${getValidatedInput("tmate-server-ed25519-fingerprint")}" \\;`; + // The regexes used here for validation are lenient, i.e. may accept + // values that are not, strictly speaking, valid, but should be good + // enough for detecting obvious errors, which is all we want here. + const options = { + "tmate-server-host": /^[a-z\d\-]+(\.[a-z\d\-]+)*$/i, + "tmate-server-port": /^\d{1,5}$/, + "tmate-server-rsa-fingerprint": /./, + "tmate-server-ed25519-fingerprint": /./, + } + + for (const opt in options) { + const value = getValidatedInput(opt, options[opt]); + if (value !== undefined) { + setDefaultCommand = `${setDefaultCommand} set-option -g ${opt} "${value}" \\;`; + } } core.debug("Creating new session") diff --git a/src/helpers.js b/src/helpers.js index 68136302..e94b27d6 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -41,11 +41,12 @@ export const execShellCommand = (cmd) => { /** * @param {string} key - * @return {string} + * @param {regex} re regex to use for validation + * @return {string}, {undefined} or throws an error if input doesn't match regex */ -export const getValidatedInput = (key) => { +export const getValidatedInput = (key, re) => { const value = core.getInput(key); - if (/^[-.+A-Za-z0-9]*$/.test(value)) { + if (value !== undefined && !re.test(value)) { throw new Error(`Invalid value for '${key}': '${value}'`); } return value; diff --git a/src/index.js b/src/index.js index 4855b32c..4bede840 100644 --- a/src/index.js +++ b/src/index.js @@ -97,11 +97,21 @@ export async function run() { await execShellCommand(`echo 'set +e' >/tmp/tmate.bashrc`); let setDefaultCommand = `set-option -g default-command "bash --rcfile /tmp/tmate.bashrc" \\;`; - if (core.getInput("tmate-server-host") !== "") { - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-host "${getValidatedInput("tmate-server-host")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-port "${getValidatedInput("tmate-server-port")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-rsa-fingerprint "${getValidatedInput("tmate-server-rsa-fingerprint")}" \\;`; - setDefaultCommand = `${setDefaultCommand} set-option -g tmate-server-ed25519-fingerprint "${getValidatedInput("tmate-server-ed25519-fingerprint")}" \\;`; + // The regexes used here for validation are lenient, i.e. may accept + // values that are not, strictly speaking, valid, but should be good + // enough for detecting obvious errors, which is all we want here. + const options = { + "tmate-server-host": /^[a-z\d\-]+(\.[a-z\d\-]+)*$/i, + "tmate-server-port": /^\d{1,5}$/, + "tmate-server-rsa-fingerprint": /./, + "tmate-server-ed25519-fingerprint": /./, + } + + for (const opt in options) { + const value = getValidatedInput(opt, options[opt]); + if (value !== undefined) { + setDefaultCommand = `${setDefaultCommand} set-option -g ${opt} "${value}" \\;`; + } } core.debug("Creating new session") diff --git a/src/index.test.js b/src/index.test.js index 658c0fcc..87e0119d 100644 --- a/src/index.test.js +++ b/src/index.test.js @@ -11,7 +11,14 @@ jest.mock("fs", () => ({ unlinkSync: () => true, writeFileSync: () => true })); -jest.mock('./helpers'); +jest.mock('./helpers', () => { + const originalModule = jest.requireActual('./helpers'); + return { + __esModule: true, + ...originalModule, + execShellCommand: jest.fn(() => 'mocked execShellCommand'), + }; +}); import { execShellCommand } from "./helpers" import { run } from "." @@ -106,4 +113,49 @@ describe('Tmate GitHub integration', () => { await run() expect(execShellCommand).not.toHaveBeenNthCalledWith(1, "brew install tmate") }); + it('should validate correct tmate options', async () => { + // Check for the happy path first. + core.getInput.mockImplementation(function(opt) { + switch (opt) { + case "tmate-server-host": return "ssh.tmate.io"; + case "tmate-server-port": return "22"; + case "tmate-server-rsa-fingerprint": return "SHA256:Hthk2T/M/Ivqfk1YYUn5ijC2Att3+UPzD7Rn72P5VWs"; + case "tmate-server-ed25519-fingerprint": return "SHA256:jfttvoypkHiQYUqUCwKeqd9d1fJj/ZiQlFOHVl6E9sI"; + default: return undefined; + } + }) + + await run() + + // Find the command launching tmate with its various options. + let tmateCmd; + for (const call of execShellCommand.mock.calls) { + const cmd = call[0] + if (cmd.includes("set-option -g")) { + tmateCmd = cmd + break + } + } + + expect(tmateCmd).toBeDefined(); + + const re = /set-option -g tmate-server-host "([^"]+)"/; + const match = re.exec(tmateCmd); + expect(match).toBeTruthy(); + expect(match[1]).toEqual("ssh.tmate.io"); + }); + it('should fail to validate wrong tmate options', async () => { + core.getInput.mockImplementation(function(opt) { + switch (opt) { + case "tmate-server-host": return "not/a/valid/hostname"; + default: return undefined; + } + }) + + await run() + + expect(core.setFailed).toHaveBeenCalledWith( + Error("Invalid value for 'tmate-server-host': 'not/a/valid/hostname'") + ) + }); });