Merge pull request #105 from vadz/fix-option-validity-check

Fix checking validity of tmate-server-xxx parameters

Author: dscho

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:

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")

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;

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")

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'")
+      )
+  });
 });