Validate and safely serialize generated harness metadata

CHANGELOG.md

@@ -4,6 +4,7 @@
 - Added `runAsync()` for CommonJS/ESM loading while preserving synchronous `run()` for CommonJS. [#59](https://github.com/lando/leia/issues/59)
 - Fixed automatic shell selection to honor Unix account shells and use explicit platform fallback precedence. [#75](https://github.com/lando/leia/issues/75)
 - Fixed generated Mocha harness failures caused by valid shell syntax in executable Markdown. [#57](https://github.com/lando/leia/issues/57)
+- Fixed generated harness validation and serialization for numeric options, paths, shell values, and identifiers. [#73](https://github.com/lando/leia/issues/73)
 - Fixed test numbering across repeated setup, test, and cleanup sections. [#75](https://github.com/lando/leia/issues/75)
 - Raised the supported Node.js engine floor from 18 to 24 and standardized development and automation on Node 24 LTS [#60](https://github.com/lando/leia/issues/60)
 

README.md

@@ -55,15 +55,15 @@ OPTIONS
   -c, --cleanup-header=cleanup-header      [default: Clean,Tear,Burn] considers these h2 sections as cleanup commands
   -i, --ignore=ignore                      files or patterns to ignore
   --module-format=auto|commonjs|esm        [default: auto] generates CommonJS or ESM harnesses, autodetected by default
-  -r, --retry=retry                        [default: 1] retries tests the given amount
+  -r, --retry=retry                        [default: 1] non-negative number of times to retry each test
   -s, --setup-header=setup-header          [default: Start,Setup,This is the dawning] considers these h2 sections as setup commands
   -t, --test-header=test-header            [default: Test,Validat,Verif] considers these h2 sections as tests
   -v, --version                            shows version info
   --debug                                  shows debug output
   --help                                   shows help
   --shell=bash|cmd|powershell|pwsh|sh|zsh  [default: /opt/homebrew/bin/zsh] runs tests with given shell, autodetected by default
   --stdin                                  attachs stdin when the test is run
-  --timeout=timeout                        [default: 1800] seconds before tests time out
+  --timeout=timeout                        [default: 1800] non-negative whole seconds before tests time out (max 2147483)
 
 EXAMPLES
   leia README.md
@@ -73,6 +73,10 @@ EXAMPLES
   leia README.md --module-format esm
 ```
 
+`--retry` and `--timeout` accept non-negative integers. Retry counts may not exceed JavaScript's safe-integer limit;
+timeouts may not exceed `2147483` seconds so their millisecond conversion remains within Node's timer range. Leia rejects
+invalid, fractional, or out-of-range values before generating or loading a harness.
+
 ### Module
 
 ```js

cli/default.js

@@ -2,6 +2,7 @@ const {Command, flags} = require('@oclif/command');
 
 const chalk = require('chalk');
 const moduleFormats = require('../lib/module-format').formats;
+const numericOption = require('../lib/numeric-option');
 const shell = require('../lib/shell.js');
 
 class LeiaCommand extends Command {
@@ -72,8 +73,9 @@ class LeiaCommand extends Command {
     }),
     'retry': flags.string({
       char: 'r',
-      description: 'retries tests the given amount',
+      description: 'non-negative number of times to retry each test',
       default: 1,
+      parse: numericOption.retry,
     }),
     'shell': flags.string({
       default: shell().binary,
@@ -84,8 +86,9 @@ class LeiaCommand extends Command {
       description: 'attachs stdin when the test is run',
     }),
     'timeout': flags.string({
-      default: '1800',
-      description: 'seconds before tests time out',
+      default: 1800,
+      description: `non-negative whole seconds before tests time out (max ${numericOption.MAX_TIMEOUT_SECONDS})`,
+      parse: numericOption.timeout,
     }),
 
     // Legacy flags that still work if you pass them in but are no longer shown in help or documented

lib/generate.js

@@ -6,6 +6,97 @@ const dot = require('dot');
 const fs = require('fs-extra');
 const path = require('path');
 
+const validateRetry = require('./numeric-option').retry;
+
+const assertString = (value, field) => {
+  if (typeof value !== 'string') {
+    throw new TypeError(`Generated harness metadata "${field}" must be a string.`);
+  }
+
+  return value;
+};
+
+const assertStringArray = (value, field) => {
+  if (!Array.isArray(value) || value.some((item) => typeof item !== 'string')) {
+    throw new TypeError(`Generated harness metadata "${field}" must be an array of strings.`);
+  }
+
+  return value;
+};
+
+const assertInteger = (value, field) => {
+  if (!Number.isSafeInteger(value) || value < 0) {
+    throw new TypeError(`Generated harness metadata "${field}" must be a non-negative safe integer.`);
+  }
+
+  return value;
+};
+
+const toSourceLiteral = (value) => JSON.stringify(value)
+  .replace(/\u2028/g, '\\u2028')
+  .replace(/\u2029/g, '\\u2029');
+
+const prepareScenario = (scenario, section, index) => {
+  const field = (name) => `tests.${section}[${index}].${name}`;
+
+  assertString(scenario.script, field('script'));
+  if (!Array.isArray(scenario.describe) || typeof scenario.describe[0] !== 'string') {
+    throw new TypeError(`Generated harness metadata "${field('describe')}" must contain a string.`);
+  }
+  if (typeof scenario.skip !== 'boolean') {
+    throw new TypeError(`Generated harness metadata "${field('skip')}" must be a boolean.`);
+  }
+
+  return {
+    args: toSourceLiteral(assertStringArray(scenario.args, field('args'))),
+    command: toSourceLiteral(assertString(scenario.command, field('command'))),
+    describe: toSourceLiteral(scenario.describe[0]),
+    id: toSourceLiteral(assertString(scenario.id, field('id'))),
+    number: toSourceLiteral(assertInteger(scenario.number, field('number'))),
+    section: toSourceLiteral(assertString(scenario.section, field('section'))),
+    shell: toSourceLiteral(assertString(scenario.shell, field('shell'))),
+    skip: scenario.skip,
+  };
+};
+
+const prepareHarness = (test) => {
+  if (!test.tests || typeof test.tests !== 'object' || Array.isArray(test.tests)) {
+    throw new TypeError('Generated harness metadata "tests" must be an object.');
+  }
+
+  const scenarios = [];
+  const tests = {};
+  _.forEach(test.tests, (sectionTests, section) => {
+    if (section === 'invalid') return;
+    if (!Array.isArray(sectionTests)) {
+      throw new TypeError(`Generated harness metadata "tests.${section}" must be an array.`);
+    }
+
+    scenarios.push(...sectionTests);
+    tests[section] = sectionTests.map((scenario, index) => prepareScenario(scenario, section, index));
+  });
+
+  const stdin = assertString(test.stdin, 'stdin');
+  if (!['inherit', 'pipe'].includes(stdin)) {
+    throw new TypeError('Generated harness metadata "stdin" must be "inherit" or "pipe".');
+  }
+
+  return {
+    renderData: {
+      chaiPath: toSourceLiteral(assertString(test.chaiPath, 'chaiPath')),
+      cltPath: toSourceLiteral(assertString(test.cltPath, 'cltPath')),
+      cwd: toSourceLiteral(assertString(test.cwd, 'cwd')),
+      debugPath: toSourceLiteral(assertString(test.debugPath, 'debugPath')),
+      id: toSourceLiteral(assertString(test.id, 'id')),
+      retry: toSourceLiteral(validateRetry(test.retry)),
+      stdin: toSourceLiteral(stdin),
+      tests,
+      version: toSourceLiteral(assertString(test.version, 'version')),
+    },
+    scenarios,
+  };
+};
+
 // Get our def files
 const getDefFiles = (dir) => _(fs.readdirSync(dir))
   .filter((file) => _.endsWith(file, '.def'))
@@ -42,6 +133,8 @@ module.exports = (tests, opts = {strip: false}) => {
     const moduleFormat = test.moduleFormat || opts.moduleFormat || 'commonjs';
     const templateFile = templates[moduleFormat];
     if (!templateFile) throw new Error(`Cannot generate unsupported module format "${moduleFormat}".`);
+    assertString(test.destination, 'destination');
+    const {renderData, scenarios} = prepareHarness(test);
 
     if (!renders[moduleFormat]) {
       debug('getting render function using template: %o and opts: %o', templateFile, opts);
@@ -53,9 +146,7 @@ module.exports = (tests, opts = {strip: false}) => {
     fs.mkdirpSync(path.dirname(test.destination));
 
     // Build and generate all our test scripts and make them executable
-    _(test.tests)
-      .filter((value, key) => key !== 'invalid')
-      .flatten()
+    _(scenarios)
       .map((data) => {
         debug('generating script to %o and making it executable', data.script);
         fs.writeFileSync(data.script, data.command);
@@ -66,7 +157,7 @@ module.exports = (tests, opts = {strip: false}) => {
 
     // Write the mocha test out
     debug('generating test %o from %o to %o', test.id, test.file, test.destination);
-    fs.writeFileSync(test.destination, renders[moduleFormat](test));
+    fs.writeFileSync(test.destination, renders[moduleFormat](renderData));
   });
 
   // Return list of tests that we can run

lib/leia.js

@@ -50,14 +50,14 @@ module.exports = class Leia {
    * @param {Array} files An array of absolute paths to markdown files
    * @param {Object} [options] An array of options
    * @param {Array} [options.cleanupHeader=['Clean']] An array of words that h2 headers can start with to be flagged as cleanup commands
-   * @param {Integer} [options.retry=3] Amount of times to retry each test
+   * @param {Integer} [options.retry=3] Non-negative safe integer amount of times to retry each test
    * @param {Array} [options.setupHeader=['Setup']] An array of words h2 headers can start with to be flagged as setup commands
    * @param {String} [options.shell=autodetected] A string containing the shell to run tests with
    * @param {Boolean} [options.stdin=false] A boolean to attach stdin or not
    * @param {Array} [options.testHeader=['Test']] An array of words h2 headers can start with to flagged as test commands
    * @param {String} [options.moduleFormat=auto] Generated harness module format: auto, commonjs, or esm
    * @return {Object} An object of parsed leia test metadate that you can use to generate mocha tests
-   * @throws {Error} When module format auto detection cannot read or parse the nearest package.json
+   * @throws {Error} When retry is invalid or module format detection cannot read or parse the nearest package.json
    */
   parse(files, options) {
     return require('./parse')(files, options);
@@ -69,8 +69,9 @@ module.exports = class Leia {
    * @since 0.5.0
    * @param {Array} tests An array of absolute paths to generated leia test files
    * @param {Object} [options] An array of Mocha options
+   * @param {Integer} [options.timeout=1800] Non-negative whole seconds, no greater than 2147483
    * @return {Object} A test loaded mocha instance
-   * @throws {Error} When tests are empty or include an ESM harness
+   * @throws {Error} When timeout is invalid, tests are empty, or tests include an ESM harness
    */
   run(tests, options) {
     return require('./run')(tests, options);
@@ -81,8 +82,9 @@ module.exports = class Leia {
    *
    * @param {Array} tests An array of absolute paths to generated Leia test files
    * @param {Object} [options] An array of Mocha options
+   * @param {Integer} [options.timeout=1800] Non-negative whole seconds, no greater than 2147483
    * @return {Promise<Object>} A promise for a loaded Mocha instance
-   * @throws {Error} When tests are empty or a harness cannot be loaded
+   * @throws {Error} When timeout is invalid, tests are empty, or a harness cannot be loaded
    */
   runAsync(tests, options) {
     return require('./run').async(tests, options);

lib/numeric-option.js

@@ -0,0 +1,21 @@
+'use strict';
+
+const MAX_RETRY = Number.MAX_SAFE_INTEGER;
+const MAX_TIMEOUT_SECONDS = Math.floor(0x7FFFFFFF / 1000);
+
+const parseNonNegativeInteger = (value, option, max) => {
+  const integer = typeof value === 'string' && /^\d+$/.test(value) ? Number(value) : value;
+
+  if (!Number.isSafeInteger(integer) || integer < 0 || integer > max) {
+    throw new Error(`${option} must be an integer between 0 and ${max}.`);
+  }
+
+  return integer;
+};
+
+module.exports = {
+  MAX_RETRY,
+  MAX_TIMEOUT_SECONDS,
+  retry: (value) => parseNonNegativeInteger(value, '--retry', MAX_RETRY),
+  timeout: (value) => parseNonNegativeInteger(value, '--timeout', MAX_TIMEOUT_SECONDS),
+};

lib/parse.js

@@ -12,6 +12,7 @@ const os = require('os');
 const path = require('path');
 
 const resolveModuleFormat = require('./module-format');
+const validateRetry = require('./numeric-option').retry;
 
 /*
  * Helper to determine whether we can whitelist a header
@@ -74,9 +75,8 @@ const parseCodeBlock = ({text, file, shell}) => {
   return _.map(text.split(`${newLine}${newLine}`), (test) => {
     const scriptDir = path.join(os.tmpdir(), 'leia', hash(file));
     const scriptPath = path.join(scriptDir, `${hash(test)}${shell.extension}`).split(path.sep).join('/');
-    const scriptArgs = `[${shell.args.map((arg) => `'${arg}'`).join(', ')}]`;
     return {
-      args: scriptArgs.replace('{0}', scriptPath),
+      args: shell.args.map((arg) => arg.replace('{0}', scriptPath)),
       shell: shell.binary.split(path.sep).join('/'),
       command: parseTestCommand(test, shell.name),
       skip: parseTestCommand(test, shell.name) === 'skip',
@@ -168,6 +168,7 @@ module.exports = (files, {
   testHeader = ['Test'],
 } = {}) => {
   const resolvedModuleFormat = resolveModuleFormat(moduleFormat, process.cwd());
+  const resolvedRetry = validateRetry(retry);
 
   return _(files)
     // Map file location into parsed markdown
@@ -181,7 +182,7 @@ module.exports = (files, {
     // Parse into more useful metadata
     .map((datum) => {
       if (datum.type === 'heading' && datum.depth === 1) {
-        const title = parseTitle(datum, {moduleFormat: resolvedModuleFormat, retry, stdin});
+        const title = parseTitle(datum, {moduleFormat: resolvedModuleFormat, retry: resolvedRetry, stdin});
         debug('found test file candidate %o with title "%o"', title.text, title.file);
         return title;
       } else return datum;

lib/run.js

@@ -6,10 +6,11 @@ const path = require('path');
 const _ = require('lodash');
 const debug = require('debug')('leia:run');
 const Mocha = require('mocha');
+const validateTimeout = require('./numeric-option').timeout;
 
 const createRunner = (tests, options) => {
   // calculate the timeout
-  const timeout = parseInt(options?.timeout ?? 1800) * 1000;
+  const timeout = validateTimeout(options?.timeout ?? 1800) * 1000;
 
   // Instantiate a Mocha instance.
   const mocha = new Mocha(_.merge({}, {timeout}));

templates/body.def

@@ -1,4 +1,4 @@
-describe('{{=it.id}}', function() {
+describe({{=it.id}}, function() {
   this.retries({{=it.retry}});
 {{~ it.tests.setup :test}}
   // These are tests we need to run to get the app into a state to test

templates/deps_commonjs.def

@@ -1,8 +1,8 @@
 
 // We need these deps to run our tezts
-const chai = require('{{=it.chaiPath}}');
-const CliTest = require('{{=it.cltPath}}');
-const debug = require('{{=it.debugPath}}')('leia:test:{{=it.id}}');
+const chai = require({{=it.chaiPath}});
+const CliTest = require({{=it.cltPath}});
+const debug = require({{=it.debugPath}})('leia:test:' + {{=it.id}});
 const path = require('path');
 chai.should();
 

templates/deps_esm.def

@@ -3,9 +3,9 @@ import {createRequire} from 'node:module';
 
 // Leia's dependencies remain CommonJS, so resolve them with Node's cross-platform CommonJS loader.
 const require = createRequire(import.meta.url);
-const chai = require('{{=it.chaiPath}}');
-const CliTest = require('{{=it.cltPath}}');
-const debug = require('{{=it.debugPath}}')('leia:test:{{=it.id}}');
+const chai = require({{=it.chaiPath}});
+const CliTest = require({{=it.cltPath}});
+const debug = require({{=it.debugPath}})('leia:test:' + {{=it.id}});
 const path = require('path');
 chai.should();
 

templates/header.def

@@ -4,19 +4,17 @@
  * See https://github.com/lando/leia for more
  * information on how all this magic works
  *
- * id: {{=it.id}}
- * runs-from: {{=it.cwd}}
  */
 
 // Set some helpful envvars so we know these are leia tezts
 process.env.LEIA = 'true';
 process.env.LEIA_ENVIRONMENT = 'true';
 process.env.LEIA_TEST_RUNNING = 'true';
-process.env.LEIA_VERSION = '{{=it.version}}';
+process.env.LEIA_VERSION = {{=it.version}};
 
 // Set legacy envars
 // These are DERECATED and will eventually be removed!!!
 process.env.LEIA_PARSER_RUNNING = 'true';
-process.env.LEIA_PARSER_VERSION = '{{=it.version}}';
-process.env.LEIA_PARSER_ID = '{{=it.id}}';
+process.env.LEIA_PARSER_VERSION = {{=it.version}};
+process.env.LEIA_PARSER_ID = {{=it.id}};
 process.env.LEIA_PARSER_RETRY = {{=it.retry}};

templates/test.def

@@ -1,14 +1,14 @@
-  it('{{=test.describe[0]}}', done => {
-    process.chdir('{{=it.cwd}}');
-    process.env.LEIA_TEST_ID = '{{=test.id}}';
-    process.env.LEIA_TEST_NUMBER = '{{=test.number}}';
+  it({{=test.describe}}, done => {
+    process.chdir({{=it.cwd}});
+    process.env.LEIA_TEST_ID = {{=test.id}};
+    process.env.LEIA_TEST_NUMBER = {{=test.number}};
     process.env.LEIA_TEST_RETRY = this.ctx.test._currentRetry;
-    process.env.LEIA_TEST_STAGE = '{{=test.section}}';
+    process.env.LEIA_TEST_STAGE = {{=test.section}};
     {{? test.skip }}this.ctx.test.skip();{{?? true }}
     const cli = new CliTest();
-    const data = {shell: '{{=test.shell}}', args: `{{=test.args}}`, commands: {{=JSON.stringify(test.command)}}, stdin: '{{=it.stdin}}'};
-    debug(`running test {{=it.id}} from {{=it.cwd}} using %o`, data);
-    cli.spawn('{{=test.shell}}', {{=test.args}}, {stdio: ['{{=it.stdin}}', 'pipe', 'pipe']}).then(res => {
+    const data = {shell: {{=test.shell}}, args: {{=test.args}}, commands: {{=test.command}}, stdin: {{=it.stdin}}};
+    debug('running test %s from %s using %o', {{=it.id}}, {{=it.cwd}}, data);
+    cli.spawn({{=test.shell}}, {{=test.args}}, {stdio: [{{=it.stdin}}, 'pipe', 'pipe']}).then(res => {
       if (res.error === null) {
         done();
       } else {