tooling: add JavaScript and Lua maintainability checks

api/eslint.config.js

@@ -20,6 +20,43 @@ export default [
       "no-new-func": "error",
       "no-console": "error",
       "no-implicit-coercion": "error",
+      "no-restricted-imports": ["error", {
+        paths: [
+          {
+            name: "node:child_process",
+            importNames: ["exec", "execSync"],
+            message: "Use spawn or spawnSync with an argument array instead of shell-based command execution.",
+          },
+          {
+            name: "child_process",
+            importNames: ["exec", "execSync"],
+            message: "Use spawn or spawnSync with an argument array instead of shell-based command execution.",
+          },
+        ],
+      }],
+      "no-restricted-properties": ["error", {
+        object: "process",
+        property: "exit",
+        message: "Set process.exitCode so pending cleanup and asynchronous work can finish.",
+      }],
+      "no-restricted-syntax": ["error",
+        {
+          selector: "CallExpression[callee.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.name='shell'][value.value=true]",
+          message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.",
+        },
+        {
+          selector: "CallExpression[callee.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.value='shell'][value.value=true]",
+          message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.",
+        },
+        {
+          selector: "CallExpression[callee.property.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.name='shell'][value.value=true]",
+          message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.",
+        },
+        {
+          selector: "CallExpression[callee.property.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.value='shell'][value.value=true]",
+          message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.",
+        },
+      ],
       "no-param-reassign": ["error", { props: true }],
       "no-shadow": "error",
       "no-var": "error",

api/tests/fixtures/activities.js

@@ -8,6 +8,9 @@ const authorDid = 'did:plc:gggggggggggggggggggggggg';
 const contributorDid = 'did:plc:hhhhhhhhhhhhhhhhhhhhhhhh';
 const indexedAt = '2025-01-02T03:04:05.000Z';
 
+/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record<string, unknown> }} SeedRow */
+
+/** @param {string} collection @param {string} rkey @param {Record<string, unknown>} fields @param {string} did @returns {Promise<SeedRow>} */
 async function row(collection, rkey, fields, did) {
   const record = { $type: collection, ...fields };
   const uri = `at://${did}/${collection}/${rkey}`;

api/tests/fixtures/actor-follows.js

@@ -6,6 +6,8 @@ const profileCollection = 'app.certified.actor.profile';
 const organizationCollection = 'app.certified.actor.organization';
 const indexedAt = '2025-01-02T03:04:05.000Z';
 
+/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record<string, unknown> }} SeedRow */
+
 export const actorFollowDids = {
   publisher: 'did:plc:aaaaaaaaaaaaaaaaaaaaaaaa',
   otherPublisher: 'did:plc:bbbbbbbbbbbbbbbbbbbbbbbb',
@@ -15,6 +17,7 @@ export const actorFollowDids = {
   curator: 'did:plc:ffffffffffffffffffffffff',
 };
 
+/** @param {string} collection @param {string} rkey @param {Record<string, unknown>} fields @param {string} did @returns {Promise<SeedRow>} */
 async function row(collection, rkey, fields, did) {
   const record = { $type: collection, ...fields };
   const uri = `at://${did}/${collection}/${rkey}`;

api/tests/fixtures/bad-dates.js

@@ -3,6 +3,10 @@
 import { encode } from '@atcute/cbor';
 import * as CID from '@atcute/cid';
 
+/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record<string, unknown> }} SeedRow */
+/** @typedef {Omit<SeedRow, 'indexedAt'> & { indexedAt: string | null; storedAt: string }} DateFixtureRow */
+
+/** @type {[string, string | number | string[] | null | undefined, string | null][]} */
 const cases = [
   ['badmissing', undefined, '2025-01-02T03:04:05.123456Z'],
   ['badnull', null, '2025-01-02T03:04:05.123456Z'],
@@ -20,6 +24,7 @@ const cases = [
   ['goodearlier', '2025-01-01T00:00:00Z', '2025-01-08T00:00:00Z'],
 ];
 
+/** @param {SeedRow} base @param {{ did: string; decorateRecord?: (record: Record<string, unknown>, rkey: string) => Record<string, unknown> }} options @returns {Promise<DateFixtureRow[]>} */
 export async function makeDateCaseRows(base, { did, decorateRecord = (record) => record }) {
   return Promise.all(cases.map(async ([rkey, createdAt, indexedAt]) => {
     const record = decorateRecord({ ...base.record }, rkey);
@@ -33,6 +38,7 @@ export async function makeDateCaseRows(base, { did, decorateRecord = (record) =>
   }));
 }
 
+/** @param {DateFixtureRow[]} rows @param {{ disposableTestTarget?: boolean }} [options] @returns {{ sql: string; params: unknown[] }[]} */
 export function badDateSeedSql(rows, { disposableTestTarget = false } = {}) {
   if (disposableTestTarget !== true) throw new Error('Bad-date seeding requires explicit disposable-test target opt-in');
   return rows.map((row) => ({

api/tests/fixtures/bad-location-dates.js

@@ -1,7 +1,10 @@
 import { makeDateCaseRows } from './bad-dates.js';
 import { locationRecords } from './records.js';
 
-export const badDateLocations = await makeDateCaseRows(locationRecords[0], {
+const [locationRecord] = locationRecords;
+if (!locationRecord) throw new Error('Bad-date fixture generation requires a location record in records.js');
+
+export const badDateLocations = await makeDateCaseRows(locationRecord, {
   did: 'did:plc:baddatefixturesexamplexx',
   decorateRecord: (record, rkey) => ({
     ...record, locationType: 'date-test', name: `Date test ${rkey}`, description: 'Synthetic timestamp fixture',

api/tests/fixtures/records.js

@@ -9,6 +9,9 @@ const collections = {
 const did = 'did:plc:abcdefghijklmnopqrstuvwx';
 const indexedAt = '2025-01-02T03:04:05.000Z';
 
+/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record<string, unknown> }} SeedRow */
+
+/** @param {string} collection @param {string} rkey @param {Record<string, unknown>} fields @param {string} [recordDid] @returns {Promise<SeedRow>} */
 async function row(collection, rkey, fields, recordDid = did) {
   const record = { $type: collection, ...fields };
   const uri = `at://${recordDid}/${collection}/${rkey}`;
@@ -84,6 +87,7 @@ export const locationRecords = await Promise.all([
   }, 'did:web:organization-only.example'),
 ]);
 
+/** @param {SeedRow[]} rows @param {{ disposableTestTarget?: boolean }} [options] @returns {{ sql: string; params: unknown[] }[]} */
 export function seedSql(rows, { disposableTestTarget = false } = {}) {
   if (disposableTestTarget !== true) throw new Error('Seeding requires explicit disposable-test target opt-in');
   if (!Array.isArray(rows) || rows.length === 0) throw new Error('Seeding requires at least one fixture row; supply a nonempty rows array');

api/tests/unit/tooling/eslint-maintainability.test.js

@@ -0,0 +1,68 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { ESLint } from 'eslint';
+import { fileURLToPath } from 'node:url';
+
+const apiRoot = fileURLToPath(new URL('../../../', import.meta.url));
+const eslint = new ESLint({ cwd: apiRoot });
+
+async function lint(source) {
+  const [result] = await eslint.lintText(source, { filePath: 'tooling/maintainability-fixture.js' });
+  return result.messages;
+}
+
+function ruleIds(messages) {
+  return messages.map(({ ruleId }) => ruleId);
+}
+
+test('rejects named synchronous child_process APIs from either Node module spelling', async () => {
+  for (const specifier of ['node:child_process', 'child_process']) {
+    for (const imported of ['exec', 'execSync']) {
+      const messages = await lint(`import { ${imported} as run } from '${specifier}'; run('tool');`);
+      assert.deepEqual(ruleIds(messages), ['no-restricted-imports'], `${specifier} must not provide ${imported}`);
+    }
+  }
+});
+
+test('rejects namespace imports from either child_process module spelling', async () => {
+  for (const specifier of ['node:child_process', 'child_process']) {
+    const messages = await lint(`import * as childProcess from '${specifier}'; childProcess.spawn('tool', []);`);
+    assert.deepEqual(ruleIds(messages), ['no-restricted-imports'], `${specifier} namespace imports are intentionally restricted`);
+  }
+});
+
+test('rejects static shell:true keys on subprocess calls but allows unrelated shell options', async () => {
+  const messages = await lint([
+    "import { spawn, spawnSync } from 'node:child_process';",
+    "spawn('tool', [], { shell: true });",
+    "spawn('tool', [], { 'shell': true });",
+    "spawn('tool', [], { ['shell']: true });",
+    "spawnSync('tool', [], { shell: true });",
+    "spawnSync('tool', [], { 'shell': true });",
+    "spawnSync('tool', [], { ['shell']: true });",
+    'const subprocess = { spawn: (...args) => args };',
+    "subprocess.spawn('tool', [], { shell: true });",
+    "subprocess.spawn('tool', [], { 'shell': true });",
+    "subprocess.spawn('tool', [], { ['shell']: true });",
+    '({ shell: true });',
+  ].join('\n'));
+
+  assert.deepEqual(ruleIds(messages), [
+    'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax',
+    'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax',
+    'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax',
+  ]);
+
+  const allowed = await lint([
+    "import { spawn } from 'node:child_process';",
+    "spawn('tool', [], { shell: false });",
+    '({ shell: true });',
+  ].join('\n'));
+  assert.deepEqual(ruleIds(allowed), []);
+});
+
+test('rejects direct and static computed process.exit while allowing process.exitCode', async () => {
+  assert.deepEqual(ruleIds(await lint('process.exit(1);')), ['no-restricted-properties']);
+  assert.deepEqual(ruleIds(await lint("process['exit'](1);")), ['no-restricted-properties']);
+  assert.deepEqual(ruleIds(await lint('process.exitCode = 1;')), []);
+});

api/tests/unit/tooling/lint-lua.test.js

@@ -1,8 +1,9 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
-import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises';
+import { access, mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises';
 import os from 'node:os';
 import path from 'node:path';
+import { buildLuaBundles } from '../../../tooling/lua-bundles.js';
 import { lintLua } from '../../../tooling/lint-lua.js';
 
 async function withTempRoot(run) {
@@ -17,17 +18,40 @@ async function withTempRoot(run) {
 async function declareLuaHandler(root, {
   manifestPath = 'modules/demo/manifest.json',
   outputPath = '../../lua/endpoints/getExample.lua',
+  sharedSourcePaths = [],
+  lexiconParameters,
 } = {}) {
   const moduleFile = path.join(root, 'modules', 'demo', 'manifest.json');
+  const sourceFile = path.join(root, 'lua', 'src', 'getExample.lua');
   await mkdir(path.dirname(moduleFile), { recursive: true });
+  await mkdir(path.dirname(sourceFile), { recursive: true });
+  try {
+    await access(sourceFile);
+  } catch (error) {
+    if (error.code !== 'ENOENT') throw error;
+    await writeFile(sourceFile, 'function handle() end\n');
+  }
   await writeFile(path.join(root, 'manifest.json'), JSON.stringify({ modules: [manifestPath] }));
-  await writeFile(moduleFile, JSON.stringify({ assets: [{
+  const assets = [];
+  if (lexiconParameters) {
+    const lexiconFile = path.join(root, 'lexicons', 'getExample.json');
+    await mkdir(path.dirname(lexiconFile), { recursive: true });
+    await writeFile(lexiconFile, JSON.stringify({
+      lexicon: 1,
+      id: 'org.example.getExample',
+      defs: { main: { type: 'query', parameters: { type: 'params', properties: Object.fromEntries(lexiconParameters.map((name) => [name, { type: 'string' }])) } } },
+    }));
+    assets.push({ kind: 'lexicon', id: 'org.example.getExample', path: '../../lexicons/getExample.json' });
+  }
+  assets.push({
     kind: 'script',
     id: 'xrpc.query:org.example.getExample',
     path: outputPath,
     sourcePath: '../../lua/src/getExample.lua',
+    sharedSourcePaths,
     config: { script_type: 'lua' },
-  }] }));
+  });
+  await writeFile(moduleFile, JSON.stringify({ assets }));
 }
 
 test('skips Lua lint when the package has no Lua files', async () => {
@@ -141,12 +165,75 @@ test('runs Luacheck only against generated handlers declared by the module manif
 
     assert.deepEqual(result, { status: 0 });
     assert.equal(command, 'luacheck');
-    assert.deepEqual(args, ['--config', '.luacheckrc', 'lua/endpoints/getExample.lua']);
+    assert.deepEqual(args, ['--config', '.luacheckrc', '--no-color', 'lua/endpoints/getExample.lua']);
     assert.equal(options.cwd, root);
-    assert.equal(options.stdio, 'inherit');
+    assert.equal(options.encoding, 'utf8');
+    assert.equal(options.stdio, 'pipe');
+  });
+});
+
+test('maps generated Luacheck locations back to authored Lua sources', async () => {
+  await withTempRoot(async (root) => {
+    const source = path.join(root, 'lua', 'src', 'getExample.lua');
+    const sharedSource = path.join(root, 'lua', 'shared', 'helper.lua');
+    const endpoint = path.join(root, 'lua', 'endpoints', 'getExample.lua');
+    await mkdir(path.dirname(source), { recursive: true });
+    await mkdir(path.dirname(sharedSource), { recursive: true });
+    await mkdir(path.dirname(endpoint), { recursive: true });
+    await writeFile(source, 'function handle()\n  missing_helper()\nend\n');
+    await writeFile(sharedSource, 'local function helper()\n  return true\nend\n');
+    await declareLuaHandler(root, { sharedSourcePaths: ['../../lua/shared/helper.lua'] });
+    await buildLuaBundles(root);
+    const output = [];
+
+    const result = await lintLua({
+      root,
+      log: (message) => output.push(message),
+      spawn: () => ({ status: 1, stdout: "lua/endpoints/getExample.lua:6:3: undefined variable 'missing_helper'\n", stderr: '' }),
+    });
+
+    assert.equal(result.status, 1);
+    assert.ok(output.some((message) => message.includes("lua/src/getExample.lua:2:3: undefined variable 'missing_helper'")));
+    assert.ok(output.some((message) => message.includes('in xrpc.query:org.example.getExample bundle at lua/endpoints/getExample.lua:6')));
   });
 });
 
+test('fails on parameter mismatches but keeps SQL interpolation diagnostics advisory', async () => {
+  const cases = [
+    {
+      source: 'function handle()\n  keys_only(params, { authors = true })\nend\n',
+      status: 1,
+      finding: /differs from its Lexicon; missing: uri; extra: authors/,
+    },
+    {
+      source: 'function handle()\n  keys_only(params, { uri = true })\n  db.raw("SELECT * FROM records WHERE uri = " .. params.uri, {})\nend\n',
+      status: 0,
+      finding: /db.raw SQL text.*directly references params/,
+    },
+  ];
+  for (const scenario of cases) {
+    await withTempRoot(async (root) => {
+      const source = path.join(root, 'lua', 'src', 'getExample.lua');
+      const endpoint = path.join(root, 'lua', 'endpoints', 'getExample.lua');
+      await mkdir(path.dirname(source), { recursive: true });
+      await mkdir(path.dirname(endpoint), { recursive: true });
+      await writeFile(source, scenario.source);
+      await writeFile(endpoint, 'function handle() end\n');
+      await declareLuaHandler(root, { lexiconParameters: ['uri'] });
+      const output = [];
+
+      const result = await lintLua({
+        root,
+        log: (message) => output.push(message),
+        spawn: () => ({ status: 0, stdout: '', stderr: '' }),
+      });
+
+      assert.equal(result.status, scenario.status);
+      assert.ok(output.some((message) => scenario.finding.test(message)));
+    });
+  }
+});
+
 test('uses the default user-local LuaRocks binary when it is not on PATH', async () => {
   await withTempRoot(async (root) => {
     const home = path.join(root, 'home');
@@ -172,7 +259,7 @@ test('uses the default user-local LuaRocks binary when it is not on PATH', async
 
     assert.deepEqual(result, { status: 0 });
     assert.deepEqual(commands.map(({ command }) => command), ['luacheck', localLuacheck]);
-    assert.deepEqual(commands[1].args, ['--config', '.luacheckrc', 'lua/endpoints/getExample.lua']);
+    assert.deepEqual(commands[1].args, ['--config', '.luacheckrc', '--no-color', 'lua/endpoints/getExample.lua']);
   });
 });