From d83fed5f828ff809a5dd5b19531ab88636a2a0bc Mon Sep 17 00:00:00 2001
From: Grahame Grieve <grahameg@gmail.com>
Date: Wed, 26 Aug 2026 14:58:18 +1000
Subject: [PATCH 1/4] Make statistics persistent  #255

---
 config-template.json       |  12 +++
 server.js                  |  24 ++---
 stats-db.js                | 127 ++++++++++++++++++++++++++
 stats.js                   | 178 +++++++++++++++++++++++++++++++++++--
 tests/server/stats.test.js | 157 ++++++++++++++++++++++++++++++++
 tx/tests/test-runner.js    |   7 +-
 tx/tx.js                   | 103 ++++++++++-----------
 7 files changed, 537 insertions(+), 71 deletions(-)
 create mode 100644 stats-db.js
 create mode 100644 tests/server/stats.test.js

diff --git a/config-template.json b/config-template.json
index 65279751..3c5295f8 100644
--- a/config-template.json
+++ b/config-template.json
@@ -6,6 +6,18 @@
       "credentials": true
     }
   },
+  // Usage statistics. Request counts are kept per module, per endpoint and per
+  // operation, and written to a SQLite database every intervalMinutes so they
+  // survive restarts and upgrades. Each row carries both the count for that
+  // interval and the all-time total. The whole block is optional.
+  "stats": {
+    // set false to keep statistics in memory only (nothing is written to disk)
+    "enabled": true,
+    // relative paths resolve under the data folder's databases directory
+    "database": "stats.db",
+    // how often the counters are written out
+    "intervalMinutes": 10
+  },
   "modules": {
     "shl": {
       "enabled": false,
diff --git a/server.js b/server.js
index bab6b842..ea2b621e 100644
--- a/server.js
+++ b/server.js
@@ -99,13 +99,13 @@ let stats = null;
 
 // Initialize modules based on configuration
 async function initializeModules() {
-  stats = new ServerStats();
+  stats = new ServerStats(config.stats, serverLog);
 
   // Initialize SHL module
   if (config.modules?.shl?.enabled) {
     try {
       serverLog.info('Initializing module: shl...');
-      modules.shl = new SHLModule(stats);
+      modules.shl = new SHLModule(stats.forModule('shl'));
       await modules.shl.initialize(config.modules.shl);
       app.use('/shl', modules.shl.router);
     } catch (error) {
@@ -118,7 +118,7 @@ async function initializeModules() {
   if (config.modules?.vcl?.enabled) {
     try {
       serverLog.info('Initializing module: vcl...');
-      modules.vcl = new VCLModule(stats);
+      modules.vcl = new VCLModule(stats.forModule('vcl'));
       await modules.vcl.initialize(config.modules.vcl);
       app.use('/VCL', modules.vcl.router);
     } catch (error) {
@@ -131,7 +131,7 @@ async function initializeModules() {
   if (config.modules?.xig?.enabled) {
     try {
       serverLog.info('Initializing module: xig...');
-      await xigModule.initializeXigModule(stats, config.modules.xig);
+      await xigModule.initializeXigModule(stats.forModule('xig'), config.modules.xig);
       app.use('/xig', xigModule.router);
       modules.xig = xigModule;
     } catch (error) {
@@ -144,7 +144,7 @@ async function initializeModules() {
   if (config.modules?.packages?.enabled) {
     try {
       serverLog.info('Initializing module: packages...');
-      modules.packages = new PackagesModule(stats);
+      modules.packages = new PackagesModule(stats.forModule('packages'));
       await modules.packages.initialize(config.modules.packages);
       app.use('/packages', modules.packages.router);
     } catch (error) {
@@ -158,7 +158,7 @@ async function initializeModules() {
   if (config.modules?.registry?.enabled) {
     try {
       serverLog.info('Initializing module: registry...');
-      modules.registry = new RegistryModule(stats);
+      modules.registry = new RegistryModule(stats.forModule('registry'));
       await modules.registry.initialize(config.modules.registry);
       app.use('/tx-reg', modules.registry.router);
     } catch (error) {
@@ -171,7 +171,7 @@ async function initializeModules() {
   if (config.modules?.publisher?.enabled) {
     try {
       serverLog.info('Initializing module: publisher...');
-      modules.publisher = new PublisherModule(stats);
+      modules.publisher = new PublisherModule(stats.forModule('publisher'));
       await modules.publisher.initialize(config.modules.publisher);
       app.use('/publisher', modules.publisher.router);
     } catch (error) {
@@ -184,7 +184,7 @@ async function initializeModules() {
   if (config.modules?.token?.enabled) {
     try {
       serverLog.info('Initializing module: token...');
-      modules.token = new TokenModule(stats);
+      modules.token = new TokenModule(stats.forModule('token'));
       await modules.token.initialize(config.modules.token);
       app.use('/token', modules.token.router);
     } catch (error) {
@@ -197,7 +197,7 @@ async function initializeModules() {
   if (config.modules?.npmprojector?.enabled) {
     try {
       serverLog.info('Initializing module: npmprojector...');
-      modules.npmprojector = new NpmProjectorModule(stats);
+      modules.npmprojector = new NpmProjectorModule(stats.forModule('npmprojector'));
       await modules.npmprojector.initialize(config.modules.npmprojector);
       const basePath = NpmProjectorModule.getBasePath(config.modules.npmprojector);
       app.use(basePath, modules.npmprojector.router);
@@ -211,7 +211,7 @@ async function initializeModules() {
   if (config.modules?.['ext-tracker']?.enabled) {
     try {
       serverLog.info('Initializing module: ext-tracker...');
-      modules.extTracker = new ExtensionTrackerModule(stats);
+      modules.extTracker = new ExtensionTrackerModule(stats.forModule('ext-tracker'));
       await modules.extTracker.initialize(config.modules['ext-tracker'], app);
     } catch (error) {
       serverLog.error('Failed to initialize extension tracker module:', error);
@@ -224,7 +224,7 @@ async function initializeModules() {
   if (config.modules?.tx?.enabled) {
     try {
       serverLog.info('Initializing module: tx...');
-      modules.tx = new TXModule(stats);
+      modules.tx = new TXModule(stats.forModule('tx'));
       await modules.tx.initialize(config.modules.tx, app);
     } catch (error) {
       serverLog.error('Failed to initialize TX module:', error);
@@ -235,7 +235,7 @@ async function initializeModules() {
   if (config.modules?.folder?.enabled) {
     try {
       serverLog.info('Initializing module: folder...');
-      modules.folder = new FolderModule(stats);
+      modules.folder = new FolderModule(stats.forModule('folder'));
       await modules.folder.initialize(config.modules.folder, app);
       // mount the router
     } catch (error) {
diff --git a/stats-db.js b/stats-db.js
new file mode 100644
index 00000000..b86cc7bb
--- /dev/null
+++ b/stats-db.js
@@ -0,0 +1,127 @@
+const path = require('path');
+const fs = require('fs');
+const Database = require('better-sqlite3');
+const folders = require('./library/folder-setup');
+
+/**
+ * Persistent storage for the server's request statistics (issue #255).
+ *
+ * The in-memory counters in ServerStats reset every time the server restarts,
+ * which makes them useless for anything but "what has happened since the last
+ * deploy". This class writes them to a SQLite database that outlives restarts
+ * and upgrades.
+ *
+ * One row per (module, endpoint, operation) per collection interval, holding
+ * both numbers:
+ *
+ *  - the *delta*: what happened in that interval. This is what you graph -
+ *    plot count_delta against time and you have requests per interval without
+ *    any differencing at the query end.
+ *  - the *total*: the all-time cumulative count, carried across restarts by
+ *    reloading it at startup. This is what you quote as "how much has this
+ *    endpoint ever been asked to do", and it means a restart shows up as a
+ *    flat spot in the graph rather than a cliff back to zero.
+ *
+ * Rows are only written for keys that saw traffic in the interval, so idle
+ * endpoints cost nothing. Nothing is pruned: at 10 minute intervals a
+ * continuously busy (endpoint, operation) pair produces ~52,000 rows a year,
+ * which SQLite does not notice.
+ *
+ * better-sqlite3 (rather than sqlite3) on purpose: the writes are synchronous,
+ * so the final flush in ServerStats.finishStats() lands on disk before
+ * server.js calls process.exit(). A handful of inserts every ten minutes is
+ * well under a millisecond, so this does not put the event loop at risk.
+ */
+class StatsDatabase {
+
+  constructor(config, log) {
+    this.log = log || console;
+    this.config = config || {};
+    this.db = null;
+    this.path = StatsDatabase.resolvePath(this.config);
+  }
+
+  /**
+   * Where the database lives: config.database if given (relative paths resolve
+   * under the data folder's databases directory), otherwise stats.db there.
+   */
+  static resolvePath(config) {
+    const configured = config && config.database;
+    if (!configured) {
+      return path.join(folders.databasesDir(), 'stats.db');
+    }
+    return path.isAbsolute(configured) ? configured : path.join(folders.databasesDir(), configured);
+  }
+
+  /**
+   * Open the database, creating the file and the schema if they aren't there.
+   */
+  open() {
+    fs.mkdirSync(path.dirname(this.path), {recursive: true});
+    this.db = new Database(this.path);
+    this.db.pragma('journal_mode = WAL');
+    this.db.exec(`
+      CREATE TABLE IF NOT EXISTS request_counts (
+        id INTEGER PRIMARY KEY AUTOINCREMENT,
+        time INTEGER NOT NULL,
+        interval_ms INTEGER NOT NULL,
+        module TEXT NOT NULL,
+        endpoint TEXT NOT NULL,
+        operation TEXT NOT NULL,
+        count_delta INTEGER NOT NULL,
+        count_total INTEGER NOT NULL,
+        time_delta INTEGER NOT NULL,
+        time_total INTEGER NOT NULL
+      )
+    `);
+    this.db.exec(`CREATE INDEX IF NOT EXISTS idx_request_counts_time ON request_counts(time)`);
+    this.db.exec(`CREATE INDEX IF NOT EXISTS idx_request_counts_key ON request_counts(module, endpoint, operation, id)`);
+
+    this.insertStatement = this.db.prepare(`
+      INSERT INTO request_counts
+        (time, interval_ms, module, endpoint, operation, count_delta, count_total, time_delta, time_total)
+      VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
+    `);
+    this.insertAll = this.db.transaction((time, intervalMs, rows) => {
+      for (const row of rows) {
+        this.insertStatement.run(time, intervalMs, row.module, row.endpoint, row.operation,
+          row.countDelta, row.countTotal, row.timeDelta, row.timeTotal);
+      }
+    });
+    return this;
+  }
+
+  /**
+   * The most recent cumulative total for each key, so counting resumes where
+   * the last run left off instead of starting again from zero.
+   */
+  loadTotals() {
+    if (!this.db) {
+      return [];
+    }
+    return this.db.prepare(`
+      SELECT module, endpoint, operation, count_total AS countTotal, time_total AS timeTotal
+      FROM request_counts
+      WHERE id IN (SELECT MAX(id) FROM request_counts GROUP BY module, endpoint, operation)
+    `).all();
+  }
+
+  /**
+   * Write one interval's worth of rows. All or nothing - if the transaction
+   * fails the caller keeps its deltas and tries again next interval.
+   */
+  write(time, intervalMs, rows) {
+    if (this.db && rows && rows.length > 0) {
+      this.insertAll(time, intervalMs, rows);
+    }
+  }
+
+  close() {
+    if (this.db) {
+      this.db.close();
+      this.db = null;
+    }
+  }
+}
+
+module.exports = StatsDatabase;
diff --git a/stats.js b/stats.js
index d5b0607d..23da6337 100644
--- a/stats.js
+++ b/stats.js
@@ -1,6 +1,49 @@
 const { monitorEventLoopDelay } = require('perf_hooks');
 const {Utilities} = require("./library/utilities");
 const escape = require('escape-html');
+const StatsDatabase = require('./stats-db');
+
+// Requests that aren't counted by a module (the root page, the dashboard) are
+// attributed here, and requests that have no endpoint dimension (everything
+// outside the tx module) carry NO_ENDPOINT.
+const SERVER_MODULE = '(server)';
+const NO_ENDPOINT = '';
+
+/**
+ * A per-module view of the server statistics.
+ *
+ * Modules are handed one of these instead of the ServerStats itself, so that
+ * everything they count is attributed to them without each of the ~76 call
+ * sites having to name its own module. The API is deliberately the same shape
+ * as ServerStats, so a module can't tell the difference - except that
+ * countRequest takes an optional endpoint, which is how the tx module
+ * distinguishes /tx/r4 from /tx/r5.
+ */
+class ModuleStats {
+
+  constructor(parent, module) {
+    this.parent = parent;
+    this.module = module;
+  }
+
+  countRequest(name, tat, endpoint = NO_ENDPOINT) {
+    this.parent.countRequest(name, tat, this.module, endpoint);
+  }
+
+  addTask(name, frequency) { this.parent.addTask(name, frequency); }
+  task(name, state) { this.parent.task(name, state); }
+  taskDone(name, state) { this.parent.taskDone(name, state); }
+  taskError(name, state) { this.parent.taskError(name, state); }
+
+  // Pass-throughs for the handful of properties modules read off the stats
+  // object. These have to be getters, not copies, or a module would capture
+  // the value at construction time and never see it change.
+  get cachingModules() { return this.parent.cachingModules; }
+  get requestCount() { return this.parent.requestCount; }
+  get staticRequestCount() { return this.parent.staticRequestCount; }
+  get history() { return this.parent.history; }
+  get startTime() { return this.parent.startTime; }
+}
 
 class ServerStats {
   started = false;
@@ -16,13 +59,53 @@ class ServerStats {
   timer;
   cachingModules = [];
   taskMap = new Map();
+  // key (module/endpoint/operation) -> counter. See countRequest().
+  counters = new Map();
+  db = null;
 
-  constructor() {
+  constructor(config, log) {
+    this.config = config || {};
+    this.log = log || console;
+    if (this.config.intervalMinutes) {
+      this.intervalMs = this.config.intervalMinutes * 60 * 1000;
+    }
+    this.openDatabase();
     this.timer = setInterval(() => {
       this.recordMetrics();
     }, this.intervalMs);
   }
 
+  /**
+   * Open the persistent counter store and pick up where the last run of the
+   * server left off. Statistics must never be the reason the server won't
+   * start, so a failure here is logged and then ignored: the counters carry
+   * on in memory, they just don't survive the next restart.
+   */
+  openDatabase() {
+    if (this.config.enabled === false) {
+      return;
+    }
+    try {
+      this.db = new StatsDatabase(this.config, this.log).open();
+      for (const row of this.db.loadTotals()) {
+        const counter = this.counterFor(row.module, row.endpoint, row.operation);
+        counter.countTotal = row.countTotal;
+        counter.timeTotal = row.timeTotal;
+      }
+    } catch (e) {
+      this.log.error(`Unable to open the statistics database: ${e.message}. Statistics will not be persisted`);
+      this.db = null;
+    }
+  }
+
+  /**
+   * A view of the statistics that attributes everything counted through it to
+   * the named module (see ModuleStats).
+   */
+  forModule(module) {
+    return new ModuleStats(this, module);
+  }
+
   recordMetrics() {
     if (this.started) {
       const now = Date.now();
@@ -61,9 +144,12 @@ class ServerStats {
       this.requestTime = 0;
       this.lastTime = now;
 
-      // Prune old data (keep 24 hours)
+      // Prune old data (keep 24 hours). Only the in-memory history is pruned -
+      // the per-endpoint counters are persisted and kept indefinitely.
       const cutoff = now - (24 * 60 * 60 * 1000); // 24 hours ago
       this.history = this.history.filter(m => m.time > cutoff);
+
+      this.flushCounters(now, this.intervalMs);
     }
   }
 
@@ -77,11 +163,78 @@ class ServerStats {
     this.recordMetrics();
   }
 
-  countRequest(name, tat) {
-    // we ignore name for now, but we might split the tat tracking up by name
-    // at some stage
+  /**
+   * The counter for one (module, endpoint, operation), created on first use.
+   *
+   * countDelta/timeDelta accumulate within the current interval and are reset
+   * each time they're written out; countTotal/timeTotal are cumulative for the
+   * life of the database, not the life of the process.
+   */
+  counterFor(module, endpoint, operation) {
+    const key = `${module}\u001f${endpoint}\u001f${operation}`;
+    let counter = this.counters.get(key);
+    if (!counter) {
+      counter = {
+        module: module, endpoint: endpoint, operation: operation,
+        countDelta: 0, timeDelta: 0, countTotal: 0, timeTotal: 0
+      };
+      this.counters.set(key, counter);
+    }
+    return counter;
+  }
+
+  /**
+   * Count one request. Modules go through their own ModuleStats, which fills
+   * in module (and, for tx, the endpoint) for them.
+   *
+   * @param {string} name - the operation, e.g. '$expand' or 'login'
+   * @param {number} tat - turnaround time in ms
+   * @param {string} module - the module that served it
+   * @param {string} endpoint - the endpoint within the module, where the
+   *   module has more than one (the tx module does; nothing else does yet)
+   */
+  countRequest(name, tat, module = SERVER_MODULE, endpoint = NO_ENDPOINT) {
     this.requestCount++;
     this.requestTime = this.requestTime + tat;
+
+    const counter = this.counterFor(module || SERVER_MODULE, endpoint || NO_ENDPOINT, name || '(unnamed)');
+    counter.countDelta++;
+    counter.countTotal++;
+    counter.timeDelta = counter.timeDelta + tat;
+    counter.timeTotal = counter.timeTotal + tat;
+  }
+
+  /**
+   * Write out the counters that saw traffic this interval, and start the next
+   * one. The deltas are only cleared if the write succeeded, so a transient
+   * database problem delays the numbers rather than losing them (the interval
+   * they're eventually written against is longer, which is what interval_ms on
+   * the row is for).
+   */
+  flushCounters(now, intervalMs) {
+    if (!this.db) {
+      return 0;
+    }
+    const rows = [];
+    for (const counter of this.counters.values()) {
+      if (counter.countDelta > 0) {
+        rows.push({...counter});
+      }
+    }
+    if (rows.length === 0) {
+      return 0;
+    }
+    try {
+      this.db.write(now, intervalMs, rows);
+    } catch (e) {
+      this.log.error(`Unable to write statistics: ${e.message}`);
+      return 0;
+    }
+    for (const counter of this.counters.values()) {
+      counter.countDelta = 0;
+      counter.timeDelta = 0;
+    }
+    return rows.length;
   }
 
   addTask(name, frequency) {
@@ -144,6 +297,17 @@ class ServerStats {
 
   finishStats() {
     clearInterval(this.timer);
+    // Synchronous, so the last partial interval is on disk before server.js
+    // calls process.exit().
+    this.flushCounters(Date.now(), Date.now() - (this.lastTime || this.startTime));
+    if (this.db) {
+      try {
+        this.db.close();
+      } catch (e) {
+        this.log.error(`Unable to close the statistics database: ${e.message}`);
+      }
+      this.db = null;
+    }
   }
 
   // Live cache aggregates across all registered caching modules (defensive: a
@@ -174,4 +338,6 @@ class ServerStats {
     }
   }
 }
-module.exports = ServerStats;
\ No newline at end of file
+module.exports = ServerStats;
+module.exports.ModuleStats = ModuleStats;
+module.exports.SERVER_MODULE = SERVER_MODULE;
\ No newline at end of file
diff --git a/tests/server/stats.test.js b/tests/server/stats.test.js
new file mode 100644
index 00000000..d3102606
--- /dev/null
+++ b/tests/server/stats.test.js
@@ -0,0 +1,157 @@
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const Database = require('better-sqlite3');
+
+const ServerStats = require('../../stats');
+
+describe('server statistics', () => {
+  let dir;
+  let dbPath;
+  let log;
+  let running;
+
+  beforeEach(() => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fhirsmith-stats-'));
+    dbPath = path.join(dir, 'stats.db');
+    log = { error: jest.fn(), warn: jest.fn(), info: jest.fn() };
+    running = [];
+  });
+
+  afterEach(() => {
+    // every ServerStats holds an interval timer, so they all have to be closed
+    // or jest won't exit
+    for (const stats of running) {
+      stats.finishStats();
+    }
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  function newStats(config) {
+    const stats = new ServerStats({ database: dbPath, ...config }, log);
+    running.push(stats);
+    return stats;
+  }
+
+  function readRows() {
+    const db = new Database(dbPath, { readonly: true });
+    try {
+      return db.prepare('SELECT * FROM request_counts ORDER BY id').all();
+    } finally {
+      db.close();
+    }
+  }
+
+  test('counts are attributed by module, endpoint and operation', () => {
+    const stats = newStats();
+    const tx = stats.forModule('tx');
+    const token = stats.forModule('token');
+
+    for (let i = 0; i < 5; i++) {
+      tx.countRequest('$expand', 10, '/tx/r4');
+    }
+    tx.countRequest('$expand', 30, '/tx/r5');
+    tx.countRequest('$validate', 7, '/tx/r4');
+    token.countRequest('login', 3);
+    stats.countRequest('dashboard', 1);
+
+    expect(stats.flushCounters(1000, 600000)).toBe(5);
+    const rows = readRows();
+
+    const r4expand = rows.find(r => r.endpoint === '/tx/r4' && r.operation === '$expand');
+    expect(r4expand.module).toBe('tx');
+    expect(r4expand.count_delta).toBe(5);
+    expect(r4expand.time_delta).toBe(50);
+    expect(rows.find(r => r.endpoint === '/tx/r5' && r.operation === '$expand').count_delta).toBe(1);
+    expect(rows.find(r => r.operation === '$validate').endpoint).toBe('/tx/r4');
+
+    // modules with a single endpoint don't carry one
+    expect(rows.find(r => r.module === 'token').endpoint).toBe('');
+    // and anything counted on the server itself is attributed to the server
+    expect(rows.find(r => r.operation === 'dashboard').module).toBe('(server)');
+
+    // the server-wide counters still see every request
+    expect(stats.requestCount).toBe(9);
+    expect(rows.every(r => r.interval_ms === 600000)).toBe(true);
+  });
+
+  test('only keys with traffic in the interval are written', () => {
+    const stats = newStats();
+    const tx = stats.forModule('tx');
+    tx.countRequest('$expand', 10, '/tx/r4');
+    tx.countRequest('$lookup', 2, '/tx/r4');
+    expect(stats.flushCounters(1000, 600000)).toBe(2);
+
+    tx.countRequest('$expand', 4, '/tx/r4');
+    expect(stats.flushCounters(2000, 600000)).toBe(1);
+    expect(stats.flushCounters(3000, 600000)).toBe(0);
+
+    expect(readRows().length).toBe(3);
+  });
+
+  test('totals survive a restart, deltas do not', () => {
+    const first = newStats();
+    first.forModule('tx').countRequest('$expand', 10, '/tx/r4');
+    first.flushCounters(1000, 600000);
+    first.finishStats();
+
+    const second = newStats();
+    second.forModule('tx').countRequest('$expand', 5, '/tx/r4');
+    second.flushCounters(2000, 600000);
+
+    const last = readRows().pop();
+    expect(last.count_delta).toBe(1);
+    expect(last.count_total).toBe(2);
+    expect(last.time_delta).toBe(5);
+    expect(last.time_total).toBe(15);
+  });
+
+  test('the last partial interval is written on shutdown', () => {
+    const stats = newStats();
+    stats.markStarted();
+    stats.forModule('tx').countRequest('$expand', 10, '/tx/r4');
+    stats.finishStats();
+
+    const rows = readRows().filter(r => r.operation === '$expand');
+    expect(rows.length).toBe(1);
+    expect(rows[0].count_total).toBe(1);
+  });
+
+  test('statistics never take the server down', () => {
+    // a database that cannot be opened is logged and then ignored. A plain
+    // file where a directory should be is a portable way to make that happen
+    const blocker = path.join(dir, 'not-a-directory');
+    fs.writeFileSync(blocker, 'x');
+    const stats = newStats({ database: path.join(blocker, 'stats.db') });
+    stats.forModule('tx').countRequest('$expand', 10, '/tx/r4');
+
+    expect(stats.db).toBeNull();
+    expect(log.error).toHaveBeenCalledWith(expect.stringContaining('Unable to open the statistics database'));
+    expect(stats.requestCount).toBe(1);
+    expect(stats.flushCounters(1000, 600000)).toBe(0);
+  });
+
+  test('statistics can be turned off', () => {
+    const stats = newStats({ enabled: false });
+    stats.forModule('tx').countRequest('$expand', 10, '/tx/r4');
+
+    expect(stats.db).toBeNull();
+    expect(fs.existsSync(dbPath)).toBe(false);
+    // still counted in memory, for the status page
+    expect(stats.requestCount).toBe(1);
+    expect(stats.counterFor('tx', '/tx/r4', '$expand').countTotal).toBe(1);
+  });
+
+  test('a module view cannot be told apart from the stats themselves', () => {
+    const stats = newStats();
+    const tx = stats.forModule('tx');
+
+    tx.addTask('Client Cache', '5 min');
+    tx.task('Client Cache', 'working');
+    expect(stats.taskDetails()).toContain('Client Cache');
+
+    const cachingModule = { expansionItemCount: () => 3 };
+    tx.cachingModules.push(cachingModule);
+    expect(stats.expansionItems()).toBe(3);
+  });
+});
diff --git a/tx/tests/test-runner.js b/tx/tests/test-runner.js
index 76ddbec6..dc31d6a8 100644
--- a/tx/tests/test-runner.js
+++ b/tx/tests/test-runner.js
@@ -104,9 +104,10 @@ async function startServer() {
     app.use(express.raw({ type: 'application/fhir+xml', limit: '50mb' }));
     app.use(express.json({ limit: '50mb' }));
 
-    // Initialize TX module only
-    stats = new ServerStats();
-    txModule = new TXModule(stats);
+    // Initialize TX module only. Statistics are kept in memory here - a test
+    // run shouldn't be writing into the real statistics database.
+    stats = new ServerStats({ enabled: false });
+    txModule = new TXModule(stats.forModule('tx'));
     await txModule.initialize(config, app);
 
     return new Promise((resolve, reject) => {
diff --git a/tx/tx.js b/tx/tx.js
index 4189f4ad..3d27707a 100644
--- a/tx/tx.js
+++ b/tx/tx.js
@@ -495,7 +495,7 @@ class TXModule {
     app.use(express.urlencoded({ extended: true }));
 
     // Set up routes
-    this.setupRoutes(router);
+    this.setupRoutes(router, endpointInfo.path);
 
     // Redirect /r5 → /r5/
     app.use((req, res, next) => {
@@ -516,8 +516,11 @@ class TXModule {
   /**
    * Set up routes for an endpoint
    * @param {express.Router} router - Express router
+   * @param {string} endpointPath - the endpoint these routes belong to, e.g.
+   *   '/tx/r4'. Captured by the handlers below so that every request is
+   *   counted against the endpoint that served it, not just the operation.
    */
-  setupRoutes(router) {
+  setupRoutes(router, endpointPath) {
     const resourceTypes = ['CodeSystem', 'ValueSet', 'ConceptMap'];
 
     // ===== Operations =====
@@ -530,7 +533,7 @@ class TXModule {
         let worker = new LookupWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$lookup', Date.now() - start);
+        this.countRequest(endpointPath, '$lookup', Date.now() - start);
       }
     });
     router.post('/CodeSystem/\\$lookup', async (req, res) => {
@@ -539,7 +542,7 @@ class TXModule {
         let worker = new LookupWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$lookup', Date.now() - start);
+        this.countRequest(endpointPath, '$lookup', Date.now() - start);
       }
     });
 
@@ -550,7 +553,7 @@ class TXModule {
         let worker = new SubsumesWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$subsumes', Date.now() - start);
+        this.countRequest(endpointPath, '$subsumes', Date.now() - start);
       }
     });
     router.post('/CodeSystem/\\$subsumes', async (req, res) => {
@@ -559,7 +562,7 @@ class TXModule {
         let worker = new SubsumesWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$subsumes', Date.now() - start);
+        this.countRequest(endpointPath, '$subsumes', Date.now() - start);
       }
     });
 
@@ -570,7 +573,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystem(req, res);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
     router.post('/CodeSystem/\\$validate-code', async (req, res) => {
@@ -579,7 +582,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystem(req, res);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
 
@@ -590,7 +593,7 @@ class TXModule {
         let worker = new BatchValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystem(req, res);
       } finally {
-        this.countRequest('$batch', Date.now() - start);
+        this.countRequest(endpointPath, '$batch', Date.now() - start);
       }
     });
     router.post('/CodeSystem/\\$batch-validate-code', async (req, res) => {
@@ -599,7 +602,7 @@ class TXModule {
         let worker = new BatchValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystem(req, res);
       } finally {
-        this.countRequest('$batch', Date.now() - start);
+        this.countRequest(endpointPath, '$batch', Date.now() - start);
       }
     });
     // ValueSet/$validate-code (GET and POST)
@@ -609,7 +612,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSet(req, res);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
     router.post('/ValueSet/\\$validate-code', async (req, res) => {
@@ -618,7 +621,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSet(req, res);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
 
@@ -629,7 +632,7 @@ class TXModule {
         let worker = new CompareWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$compare', Date.now() - start);
+        this.countRequest(endpointPath, '$compare', Date.now() - start);
       }
     });
     router.post('/ValueSet/\\$compare', async (req, res) => {
@@ -638,7 +641,7 @@ class TXModule {
         let worker = new CompareWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$compare', Date.now() - start);
+        this.countRequest(endpointPath, '$compare', Date.now() - start);
       }
     });
 
@@ -649,7 +652,7 @@ class TXModule {
         let worker = new BatchValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSet(req, res);
       } finally {
-        this.countRequest('$batch', Date.now() - start);
+        this.countRequest(endpointPath, '$batch', Date.now() - start);
       }
     });
     router.post('/ValueSet/\\$batch-validate-code', async (req, res) => {
@@ -658,7 +661,7 @@ class TXModule {
         let worker = new BatchValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSet(req, res);
       } finally {
-        this.countRequest('validate', Date.now() - start);
+        this.countRequest(endpointPath, 'validate', Date.now() - start);
       }
     });
 
@@ -669,7 +672,7 @@ class TXModule {
         let worker = new ExpandWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n, this.internalLimit(req), this.externalLimit(req));
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$expand', Date.now() - start);
+        this.countRequest(endpointPath, '$expand', Date.now() - start);
       }
     });
     router.post('/ValueSet/\\$expand', async (req, res) => {
@@ -678,7 +681,7 @@ class TXModule {
         let worker = new ExpandWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n, this.internalLimit(req), this.externalLimit(req));
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$expand', Date.now() - start);
+        this.countRequest(endpointPath, '$expand', Date.now() - start);
       }
     });
 
@@ -689,7 +692,7 @@ class TXModule {
         let worker = new CacheControlWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$cache-control', Date.now() - start);
+        this.countRequest(endpointPath, '$cache-control', Date.now() - start);
       }
     });
     router.post('/\\$cache-control', async (req, res) => {
@@ -698,7 +701,7 @@ class TXModule {
         let worker = new CacheControlWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$cache-control', Date.now() - start);
+        this.countRequest(endpointPath, '$cache-control', Date.now() - start);
       }
     });
 
@@ -709,7 +712,7 @@ class TXModule {
         let worker = new TranslateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$translate', Date.now() - start);
+        this.countRequest(endpointPath, '$translate', Date.now() - start);
       }
     });
     router.post('/ConceptMap/\\$translate', async (req, res) => {
@@ -718,7 +721,7 @@ class TXModule {
         let worker = new TranslateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$translate', Date.now() - start);
+        this.countRequest(endpointPath, '$translate', Date.now() - start);
       }
     });
 
@@ -729,7 +732,7 @@ class TXModule {
         let worker = new ClosureWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$closure', Date.now() - start);
+        this.countRequest(endpointPath, '$closure', Date.now() - start);
       }
     });
     router.post('/ConceptMap/\\$closure', async (req, res) => {
@@ -738,7 +741,7 @@ class TXModule {
         let worker = new ClosureWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res, this.log);
       } finally {
-        this.countRequest('$closure', Date.now() - start);
+        this.countRequest(endpointPath, '$closure', Date.now() - start);
       }
     });
 
@@ -751,7 +754,7 @@ class TXModule {
         let worker = new LookupWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res);
       } finally {
-        this.countRequest('$lookup', Date.now() - start);
+        this.countRequest(endpointPath, '$lookup', Date.now() - start);
       }
     });
     router.post('/CodeSystem/:id/\\$lookup', async (req, res) => {
@@ -760,7 +763,7 @@ class TXModule {
         let worker = new LookupWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res);
       } finally {
-        this.countRequest('$lookup', Date.now() - start);
+        this.countRequest(endpointPath, '$lookup', Date.now() - start);
       }
     });
 
@@ -771,7 +774,7 @@ class TXModule {
         let worker = new SubsumesWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res);
       } finally {
-        this.countRequest('$subsumes', Date.now() - start);
+        this.countRequest(endpointPath, '$subsumes', Date.now() - start);
       }
     });
     router.post('/CodeSystem/:id/\\$subsumes', async (req, res) => {
@@ -780,7 +783,7 @@ class TXModule {
         let worker = new SubsumesWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res);
       } finally {
-        this.countRequest('$subsumes', Date.now() - start);
+        this.countRequest(endpointPath, '$subsumes', Date.now() - start);
       }
     });
 
@@ -791,7 +794,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystemInstance(req, res, this.log);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
     router.post('/CodeSystem/:id/\\$validate-code', async (req, res) => {
@@ -800,7 +803,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleCodeSystemInstance(req, res, this.log);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
 
     });
@@ -812,7 +815,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSetInstance(req, res, this.log);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
     router.post('/ValueSet/:id/\\$validate-code', async (req, res) => {
@@ -821,7 +824,7 @@ class TXModule {
         let worker = new ValidateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleValueSetInstance(req, res, this.log);
       } finally {
-        this.countRequest('$validate', Date.now() - start);
+        this.countRequest(endpointPath, '$validate', Date.now() - start);
       }
     });
 
@@ -833,7 +836,7 @@ class TXModule {
         let worker = new CompareWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$compare', Date.now() - start);
+        this.countRequest(endpointPath, '$compare', Date.now() - start);
       }
     });
     router.post('/ValueSet/:id/\\$compare', async (req, res) => {
@@ -842,7 +845,7 @@ class TXModule {
         let worker = new CompareWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$compare', Date.now() - start);
+        this.countRequest(endpointPath, '$compare', Date.now() - start);
       }
     });
 
@@ -853,7 +856,7 @@ class TXModule {
         let worker = new ExpandWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n, this.internalLimit(req), this.externalLimit(req));
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$expand', Date.now() - start);
+        this.countRequest(endpointPath, '$expand', Date.now() - start);
       }
     });
     router.post('/ValueSet/:id/\\$expand', async (req, res) => {
@@ -862,7 +865,7 @@ class TXModule {
         let worker = new ExpandWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n, this.internalLimit(req), this.externalLimit(req));
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$expand', Date.now() - start);
+        this.countRequest(endpointPath, '$expand', Date.now() - start);
       }
     });
 
@@ -873,7 +876,7 @@ class TXModule {
         let worker = new TranslateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$translate', Date.now() - start);
+        this.countRequest(endpointPath, '$translate', Date.now() - start);
       }
     });
     router.post('/ConceptMap/:id/\\$translate', async (req, res) => {
@@ -882,7 +885,7 @@ class TXModule {
         let worker = new TranslateWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handleInstance(req, res, this.log);
       } finally {
-        this.countRequest('$translate', Date.now() - start);
+        this.countRequest(endpointPath, '$translate', Date.now() - start);
       }
     });
 
@@ -904,7 +907,7 @@ class TXModule {
           let worker = new ReadWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
           await worker.handle(req, res, resourceType);
         } finally {
-          this.countRequest('read', Date.now() - start);
+          this.countRequest(endpointPath, 'read', Date.now() - start);
         }
       });
     }
@@ -917,7 +920,7 @@ class TXModule {
           let worker = new SearchWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
           await worker.handle(req, res, resourceType);
         } finally {
-          this.countRequest('search', Date.now() - start);
+          this.countRequest(endpointPath, 'search', Date.now() - start);
         }
       });
       router.post(`/${resourceType}/_search`, async (req, res) => {
@@ -926,7 +929,7 @@ class TXModule {
           let worker = new SearchWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
           await worker.handle(req, res, resourceType);
         } finally {
-          this.countRequest('search', Date.now() - start);
+          this.countRequest(endpointPath, 'search', Date.now() - start);
         }
       });
     }
@@ -944,7 +947,7 @@ class TXModule {
             ));
           }
         } finally {
-          this.countRequest('$read', Date.now() - start);
+          this.countRequest(endpointPath, '$read', Date.now() - start);
         }
       });
     }
@@ -955,7 +958,7 @@ class TXModule {
         let worker = new OperationsWorker(req.txOpContext, this.log, req.txProvider, this.languages, this.i18n);
         await worker.handle(req, res);
       } finally {
-        this.countRequest('$op', Date.now() - start);
+        this.countRequest(endpointPath, '$op', Date.now() - start);
       }
     });
 
@@ -969,7 +972,7 @@ class TXModule {
         res.setHeader('Content-Type', 'text/html');
         res.send(html);
       } finally {
-        this.countRequest('problems', Date.now() - start);
+        this.countRequest(endpointPath, 'problems', Date.now() - start);
       }
     });
 
@@ -984,7 +987,7 @@ class TXModule {
           res.status(500).json(this.operationOutcome('error', 'exception', error.message));
         }
       } finally {
-        this.countRequest('metadata', Date.now() - start);
+        this.countRequest(endpointPath, 'metadata', Date.now() - start);
       }
     });
 
@@ -999,7 +1002,7 @@ class TXModule {
           res.status(500).json(this.operationOutcome('error', 'exception', error.message));
         }
       } finally {
-        this.countRequest('$versions', Date.now() - start);
+        this.countRequest(endpointPath, '$versions', Date.now() - start);
       }
     });
 
@@ -1016,7 +1019,7 @@ class TXModule {
           }]
         });
       } finally {
-        this.countRequest('home', Date.now() - start);
+        this.countRequest(endpointPath, 'home', Date.now() - start);
       }
     });
 
@@ -1041,7 +1044,7 @@ class TXModule {
         this.log.error(`Error rendering info page for ${req.params.id}: ${error.message}`);
         res.status(500).send('Internal server error');
       } finally {
-        this.countRequest('info', Date.now() - start);
+        this.countRequest(endpointPath, 'info', Date.now() - start);
       }
     };
     router.get('/info/:id', infoHandler);
@@ -1218,9 +1221,9 @@ class TXModule {
     return data;
   }
 
-  countRequest(name, tat) {
+  countRequest(endpoint, name, tat) {
     if (this.stats) {
-      this.stats.countRequest(name, tat);
+      this.stats.countRequest(name, tat, endpoint);
     }
   }
 

From 9a40e97e7f09aea2446623204df8d47f16c73226 Mon Sep 17 00:00:00 2001
From: Grahame Grieve <grahameg@gmail.com>
Date: Wed, 26 Aug 2026 16:01:05 +1000
Subject: [PATCH 2/4] validate input fields when publishing

---
 publisher/publisher.js             |  20 ++-
 publisher/validation.js            | 198 +++++++++++++++++++++++++++++
 tests/publisher/validation.test.js | 165 ++++++++++++++++++++++++
 3 files changed, 378 insertions(+), 5 deletions(-)
 create mode 100644 publisher/validation.js
 create mode 100644 tests/publisher/validation.test.js

diff --git a/publisher/publisher.js b/publisher/publisher.js
index 52945916..803ce687 100644
--- a/publisher/publisher.js
+++ b/publisher/publisher.js
@@ -1,6 +1,7 @@
 const express = require('express');
 const path = require('path');
 const fs = require('fs');
+const validation = require('./validation');
 const Database = require('sqlite3').Database;
 const bcrypt = require('bcrypt');
 const session = require('express-session');
@@ -1501,23 +1502,23 @@ class PublisherModule {
           content += '</div>';
           content += '<div class="col-md-3">';
           content += '<label for="github_org" class="form-label">GitHub Org</label>';
-          content += '<input type="text" class="form-control" id="github_org" name="github_org" required placeholder="hl7">';
+          content += '<input type="text" class="form-control" id="github_org" name="github_org" required maxlength="39" pattern="' + validation.HTML_PATTERNS.github_org + '" title="Letters, digits and single hyphens" placeholder="hl7">';
           content += '</div>';
           content += '<div class="col-md-3">';
           content += '<label for="github_repo" class="form-label">GitHub Repo</label>';
-          content += '<input type="text" class="form-control" id="github_repo" name="github_repo" required placeholder="fhir-us-core">';
+          content += '<input type="text" class="form-control" id="github_repo" name="github_repo" required maxlength="100" pattern="' + validation.HTML_PATTERNS.github_repo + '" title="Letters, digits, dots, hyphens and underscores" placeholder="fhir-us-core">';
           content += '</div>';
           content += '<div class="col-md-3">';
           content += '<label for="git_branch" class="form-label">Branch</label>';
-          content += '<input type="text" class="form-control" id="git_branch" name="git_branch" required placeholder="main">';
+          content += '<input type="text" class="form-control" id="git_branch" name="git_branch" required maxlength="255" pattern="' + validation.HTML_PATTERNS.git_branch + '" title="A git branch name - slashes are fine, but not spaces, backslashes or any of ~ ^ : ? * [" placeholder="main">';
           content += '</div>';
           content += '<div class="col-md-4">';
           content += '<label for="npm_package_id" class="form-label">NPM Package ID</label>';
-          content += '<input type="text" class="form-control" id="npm_package_id" name="npm_package_id" required placeholder="hl7.fhir.us.core">';
+          content += '<input type="text" class="form-control" id="npm_package_id" name="npm_package_id" required maxlength="128" pattern="' + validation.HTML_PATTERNS.npm_package_id + '" title="Letters, digits, dots, hyphens and underscores" placeholder="hl7.fhir.us.core">';
           content += '</div>';
           content += '<div class="col-md-4">';
           content += '<label for="version" class="form-label">Version</label>';
-          content += '<input type="text" class="form-control" id="version" name="version" required placeholder="6.0.0">';
+          content += '<input type="text" class="form-control" id="version" name="version" required maxlength="64" pattern="' + validation.HTML_PATTERNS.version + '" title="Letters, digits, dots, hyphens and plus signs" placeholder="6.0.0">';
           content += '</div>';
           content += '<div class="col-md-4 d-flex align-items-end">';
           content += '<button type="submit" class="btn btn-primary">Create Task</button>';
@@ -1633,6 +1634,15 @@ class PublisherModule {
       try {
         const {website_id, github_org, github_repo, git_branch, npm_package_id, version} = req.body;
 
+        // Check the input before it goes anywhere near a git command line or a
+        // file name. The browser checks these too (see HTML_PATTERNS), but the
+        // form isn't the only way to reach this route.
+        const problems = validation.validateTaskInput(req.body);
+        if (problems.length > 0) {
+          this.logger.warn('Rejected task creation from user ' + req.session.userId + ': ' + problems.join('; '));
+          return res.status(400).send('Invalid task details: ' + problems.join('; '));
+        }
+
         // Verify user has permission for this website
         const canQueue = await this.userCanQueue(req.session.userId, website_id);
         if (!canQueue) {
diff --git a/publisher/validation.js b/publisher/validation.js
new file mode 100644
index 00000000..3156929a
--- /dev/null
+++ b/publisher/validation.js
@@ -0,0 +1,198 @@
+/**
+ * Validation for the fields a user types into the publication task form.
+ *
+ * None of these values can cause command injection - they're passed to spawn()
+ * as argv, and nothing goes near a shell - but they do end up in git command
+ * lines, in URLs, and in file names, so they're checked at the boundary rather
+ * than trusted to be harmless everywhere they're later used.
+ *
+ * The rules are the ones GitHub and git themselves enforce (see
+ * git-check-ref-format(1) for the branch rules): anything rejected here would
+ * have failed at clone time anyway, with a much worse error.
+ */
+
+// GitHub owner names: letters, digits and single hyphens, no hyphen at either
+// end, 39 characters at most.
+const MAX_OWNER = 39;
+// Repository names: letters, digits, dot, hyphen, underscore, 100 at most.
+const MAX_REPO = 100;
+// git imposes no length limit on a ref, but a branch name past this is a
+// filesystem problem waiting to happen.
+const MAX_BRANCH = 255;
+const MAX_PACKAGE_ID = 128;
+const MAX_VERSION = 64;
+
+/**
+ * A GitHub organisation or user name.
+ */
+function validateGithubOwner(value) {
+  if (!value) {
+    return 'GitHub org is required';
+  }
+  if (value.length > MAX_OWNER) {
+    return `GitHub org must be ${MAX_OWNER} characters or less`;
+  }
+  if (!/^[A-Za-z0-9](?:-?[A-Za-z0-9])*$/.test(value)) {
+    return 'GitHub org may only contain letters, digits and single hyphens, and may not start or end with a hyphen';
+  }
+  return null;
+}
+
+/**
+ * A GitHub repository name.
+ */
+function validateGithubRepo(value) {
+  if (!value) {
+    return 'GitHub repo is required';
+  }
+  if (value.length > MAX_REPO) {
+    return `GitHub repo must be ${MAX_REPO} characters or less`;
+  }
+  if (!/^[A-Za-z0-9._-]+$/.test(value)) {
+    return 'GitHub repo may only contain letters, digits, dots, hyphens and underscores';
+  }
+  if (value === '.' || value === '..') {
+    return 'GitHub repo is not a valid name';
+  }
+  // the code appends '.git' when it builds the clone URL, and GitHub won't
+  // create a repository with that suffix anyway
+  if (value.toLowerCase().endsWith('.git')) {
+    return 'GitHub repo should not include the .git suffix';
+  }
+  return null;
+}
+
+/**
+ * A branch name, by the rules in git-check-ref-format(1). Slashes are fine -
+ * that's how branches are grouped - but the characters git reserves for
+ * revision syntax are not. Note that git itself rejects a backslash in a ref
+ * name, so this does too.
+ */
+function validateGitBranch(value) {
+  if (!value) {
+    return 'Branch is required';
+  }
+  if (value.length > MAX_BRANCH) {
+    return `Branch must be ${MAX_BRANCH} characters or less`;
+  }
+  // space, the ASCII control characters, and DEL
+  // eslint-disable-next-line no-control-regex
+  if (/[\u0000-\u001F\u007F ]/.test(value)) {
+    return 'Branch may not contain spaces or control characters';
+  }
+  // the characters git reserves for revision syntax
+  if (/[~^:?*[\\]/.test(value)) {
+    return 'Branch may not contain any of ~ ^ : ? * [ \\';
+  }
+  if (value.includes('..')) {
+    return 'Branch may not contain ..';
+  }
+  if (value.includes('@{')) {
+    return 'Branch may not contain @{';
+  }
+  if (value === '@') {
+    return 'Branch may not be @';
+  }
+  if (value.startsWith('-')) {
+    return 'Branch may not start with a hyphen';
+  }
+  if (value.startsWith('/') || value.endsWith('/') || value.includes('//')) {
+    return 'Branch may not start or end with /, or contain //';
+  }
+  if (value.endsWith('.')) {
+    return 'Branch may not end with .';
+  }
+  for (const part of value.split('/')) {
+    if (part.startsWith('.')) {
+      return 'No part of a branch name may start with .';
+    }
+    if (part.endsWith('.lock')) {
+      return 'No part of a branch name may end with .lock';
+    }
+  }
+  return null;
+}
+
+/**
+ * An NPM package id. This one is not about git at all: the package id and the
+ * version are concatenated into file names under the zips directory
+ * (<id>#<version>.log, <id>#<version>-announcement.txt), so a value containing
+ * a path separator would look outside that directory.
+ */
+function validatePackageId(value) {
+  if (!value) {
+    return 'NPM package id is required';
+  }
+  if (value.length > MAX_PACKAGE_ID) {
+    return `NPM package id must be ${MAX_PACKAGE_ID} characters or less`;
+  }
+  if (!/^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(value)) {
+    return 'NPM package id may only contain letters, digits, dots, hyphens and underscores, and must start with a letter or digit';
+  }
+  if (value.includes('..')) {
+    return 'NPM package id may not contain ..';
+  }
+  return null;
+}
+
+/**
+ * A package version. Loose enough for semver with pre-release and build
+ * metadata, tight enough that it can't become a path.
+ */
+function validateVersion(value) {
+  if (!value) {
+    return 'Version is required';
+  }
+  if (value.length > MAX_VERSION) {
+    return `Version must be ${MAX_VERSION} characters or less`;
+  }
+  if (!/^[A-Za-z0-9][A-Za-z0-9.+-]*$/.test(value)) {
+    return 'Version may only contain letters, digits, dots, hyphens and plus signs, and must start with a letter or digit';
+  }
+  if (value.includes('..')) {
+    return 'Version may not contain ..';
+  }
+  return null;
+}
+
+/**
+ * Validate everything the task form collects.
+ * @returns {string[]} the problems found, empty if the input is acceptable
+ */
+function validateTaskInput(input) {
+  const errors = [];
+  const checks = [
+    validateGithubOwner(input.github_org),
+    validateGithubRepo(input.github_repo),
+    validateGitBranch(input.git_branch),
+    validatePackageId(input.npm_package_id),
+    validateVersion(input.version)
+  ];
+  for (const error of checks) {
+    if (error) {
+      errors.push(error);
+    }
+  }
+  return errors;
+}
+
+// Patterns for the HTML form, so the browser objects before the round trip.
+// Deliberately a subset of the checks above - the server is what decides -
+// but they catch the obvious mistakes as the user types.
+const HTML_PATTERNS = {
+  github_org: '[A-Za-z0-9](-?[A-Za-z0-9])*',
+  github_repo: '[A-Za-z0-9._-]+',
+  git_branch: '[^\\\\ ~^:?*\\[]+',
+  npm_package_id: '[A-Za-z0-9][A-Za-z0-9._-]*',
+  version: '[A-Za-z0-9][A-Za-z0-9.+-]*'
+};
+
+module.exports = {
+  validateGithubOwner,
+  validateGithubRepo,
+  validateGitBranch,
+  validatePackageId,
+  validateVersion,
+  validateTaskInput,
+  HTML_PATTERNS
+};
diff --git a/tests/publisher/validation.test.js b/tests/publisher/validation.test.js
new file mode 100644
index 00000000..9f370c03
--- /dev/null
+++ b/tests/publisher/validation.test.js
@@ -0,0 +1,165 @@
+const {
+  validateGithubOwner,
+  validateGithubRepo,
+  validateGitBranch,
+  validatePackageId,
+  validateVersion,
+  validateTaskInput,
+  HTML_PATTERNS
+} = require('../../publisher/validation');
+
+describe('publication task input validation', () => {
+
+  describe('github org', () => {
+    test.each(['hl7', 'HL7', 'fhir-org', 'a', 'a1', 'x'.repeat(39)])('accepts %s', (value) => {
+      expect(validateGithubOwner(value)).toBeNull();
+    });
+
+    test.each([
+      ['', 'empty'],
+      ['-hl7', 'leading hyphen'],
+      ['hl7-', 'trailing hyphen'],
+      ['hl7--org', 'double hyphen'],
+      ['hl7/other', 'slash'],
+      ['hl7.org', 'dot'],
+      ['hl7 org', 'space'],
+      ['hl7;whoami', 'semicolon'],
+      ['x'.repeat(40), 'too long']
+    ])('rejects %s (%s)', (value) => {
+      expect(validateGithubOwner(value)).not.toBeNull();
+    });
+  });
+
+  describe('github repo', () => {
+    test.each(['fhir-us-core', 'ig.registry', 'my_repo', 'a', 'x'.repeat(100)])('accepts %s', (value) => {
+      expect(validateGithubRepo(value)).toBeNull();
+    });
+
+    test.each([
+      ['', 'empty'],
+      ['.', 'dot'],
+      ['..', 'parent'],
+      ['a/b', 'slash'],
+      ['a b', 'space'],
+      ['a;b', 'semicolon'],
+      ['$(whoami)', 'command substitution'],
+      ['a|b', 'pipe'],
+      ['repo.git', '.git suffix'],
+      ['x'.repeat(101), 'too long']
+    ])('rejects %s (%s)', (value) => {
+      expect(validateGithubRepo(value)).not.toBeNull();
+    });
+  });
+
+  describe('git branch', () => {
+    // slashes group branches, so they have to keep working
+    test.each([
+      'main',
+      'master',
+      'release/6.0.0',
+      'feature/JIRA-123_thing',
+      '2026-08-gg-tx-fixes',
+      'v1.0.0',
+      'a/b/c/d'
+    ])('accepts %s', (value) => {
+      expect(validateGitBranch(value)).toBeNull();
+    });
+
+    // every one of these is rejected by git check-ref-format too
+    test.each([
+      ['', 'empty'],
+      ['has space', 'space'],
+      ['tilde~1', 'tilde'],
+      ['caret^1', 'caret'],
+      ['colon:name', 'colon'],
+      ['question?', 'question mark'],
+      ['star*', 'asterisk'],
+      ['bracket[', 'open bracket'],
+      ['back\\slash', 'backslash'],
+      ['a..b', 'double dot'],
+      ['main@{0}', 'reflog syntax'],
+      ['@', 'bare at'],
+      ['-lead', 'leading hyphen'],
+      ['/lead', 'leading slash'],
+      ['trail/', 'trailing slash'],
+      ['a//b', 'double slash'],
+      ['.hidden', 'leading dot'],
+      ['a/.hidden', 'component with leading dot'],
+      ['ends.', 'trailing dot'],
+      ['a/b.lock', 'lock suffix'],
+      ['bell\u0007', 'control character'],
+      ['x'.repeat(256), 'too long']
+    ])('rejects %s (%s)', (value) => {
+      expect(validateGitBranch(value)).not.toBeNull();
+    });
+  });
+
+  describe('package id and version', () => {
+    test.each(['hl7.fhir.us.core', 'hl7.fhir.r4.core', 'my_package-1'])('accepts package id %s', (value) => {
+      expect(validatePackageId(value)).toBeNull();
+    });
+
+    // these are the ones that matter: the id and version are concatenated into
+    // file names under the zips directory
+    test.each(['../../etc/passwd', 'a/b', 'a..b', '.hidden', 'a b', ''])('rejects package id %s', (value) => {
+      expect(validatePackageId(value)).not.toBeNull();
+    });
+
+    test.each(['6.0.0', '1.0.0-ballot', '2.1.0-snapshot.3', '1.0.0+build7'])('accepts version %s', (value) => {
+      expect(validateVersion(value)).toBeNull();
+    });
+
+    test.each(['../6.0.0', '6/0/0', '6..0', '6 0', ''])('rejects version %s', (value) => {
+      expect(validateVersion(value)).not.toBeNull();
+    });
+  });
+
+  describe('the form as a whole', () => {
+    const good = {
+      github_org: 'hl7',
+      github_repo: 'fhir-us-core',
+      git_branch: 'release/6.0.0',
+      npm_package_id: 'hl7.fhir.us.core',
+      version: '6.0.0'
+    };
+
+    test('accepts a realistic task', () => {
+      expect(validateTaskInput(good)).toEqual([]);
+    });
+
+    test('reports every bad field, not just the first', () => {
+      expect(validateTaskInput({
+        github_org: '-bad-',
+        github_repo: 'a;b',
+        git_branch: 'x..y',
+        npm_package_id: '../../etc/passwd',
+        version: '1.0;0'
+      })).toHaveLength(5);
+    });
+
+    test('rejects missing fields', () => {
+      expect(validateTaskInput({})).toHaveLength(5);
+    });
+  });
+
+  describe('html patterns', () => {
+    // the browser-side patterns must not reject anything the server accepts,
+    // or the form becomes impossible to submit
+    test.each([
+      ['github_org', 'hl7'],
+      ['github_repo', 'fhir-us-core'],
+      ['git_branch', 'release/6.0.0'],
+      ['npm_package_id', 'hl7.fhir.us.core'],
+      ['version', '6.0.0-ballot+1']
+    ])('%s pattern accepts %s', (field, value) => {
+      expect(new RegExp('^(?:' + HTML_PATTERNS[field] + ')$').test(value)).toBe(true);
+    });
+
+    test('the branch pattern still catches the obvious ones', () => {
+      const re = new RegExp('^(?:' + HTML_PATTERNS.git_branch + ')$');
+      for (const bad of ['has space', 'a~b', 'a^b', 'a:b', 'a?b', 'a*b', 'a[b', 'back\\slash']) {
+        expect(re.test(bad)).toBe(false);
+      }
+    });
+  });
+});

From 3a66a13db3933f0c757832185c16d11f7111e2a9 Mon Sep 17 00:00:00 2001
From: Grahame Grieve <grahameg@gmail.com>
Date: Thu, 27 Aug 2026 12:30:45 +1000
Subject: [PATCH 3/4] tighten up on $lookup parameters value types and
 character sets

---
 tests/tx/parameter-check.test.js | 147 +++++++++++++++++++++++++++++++
 tests/tx/xml.test.js             |  25 ++++++
 tx/library/parameter-check.js    | 139 +++++++++++++++++++++++++++++
 tx/workers/lookup.js             |  15 +++-
 tx/xml/xml-base.js               |  22 ++++-
 5 files changed, 346 insertions(+), 2 deletions(-)
 create mode 100644 tests/tx/parameter-check.test.js
 create mode 100644 tx/library/parameter-check.js

diff --git a/tests/tx/parameter-check.test.js b/tests/tx/parameter-check.test.js
new file mode 100644
index 00000000..bc67c5fb
--- /dev/null
+++ b/tests/tx/parameter-check.test.js
@@ -0,0 +1,147 @@
+/**
+ * The check applied to whatever a code system provider adds to a $lookup
+ * response in extendLookup().
+ */
+const { checkAddedParameters } = require('../../tx/library/parameter-check');
+
+const SOURCE = 'http://example.org/CodeSystem/test';
+
+function check(params, fromIndex = 0) {
+  return () => checkAddedParameters(params, fromIndex, SOURCE);
+}
+
+describe('lookup parameter checking', () => {
+
+  describe('values a provider may add', () => {
+    test.each([
+      ['valueString', 'a string'],
+      ['valueCode', 'active'],
+      ['valueBoolean', true],
+      ['valueBoolean', false],
+      ['valueInteger', 42],
+      ['valueInteger', 0],
+      ['valueDecimal', 1.5],
+      ['valueDateTime', '2026-08-26T10:00:00Z'],
+      ['valueUri', 'http://example.org'],
+      ['valueCanonical', 'http://example.org/vs|1.0.0']
+    ])('accepts %s', (key, value) => {
+      expect(check([{ name: 'property', part: [{ name: 'code', valueCode: 'x' }, { name: 'value', [key]: value }] }])).not.toThrow();
+    });
+
+    test('accepts valueCoding, the one complex type', () => {
+      expect(check([{
+        name: 'property',
+        part: [
+          { name: 'code', valueCode: 'parent' },
+          { name: 'value', valueCoding: { system: 'http://snomed.info/sct', code: '73211009', display: 'Diabetes' } }
+        ]
+      }])).not.toThrow();
+    });
+
+    test('accepts the designation shape the worker itself builds', () => {
+      expect(check([{
+        name: 'designation',
+        part: [
+          { name: 'language', valueCode: 'en' },
+          { name: 'use', valueCoding: { system: 'http://snomed.info/sct', code: '900000000000013009' } },
+          { name: 'value', valueString: 'Diabetes mellitus' }
+        ]
+      }])).not.toThrow();
+    });
+  });
+
+  describe('values it may not', () => {
+    // this is the case that prompted the check: a remote API hands back
+    // arbitrary JSON and it lands in a FHIR primitive
+    test('rejects an object where a string belongs', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueString: { nested: 'thing' } }] }]))
+        .toThrow(/valueString must be a string, not a object/);
+    });
+
+    test('rejects an array where a string belongs', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueString: ['a', 'b'] }] }]))
+        .toThrow(/valueString must be a string, not an array/);
+    });
+
+    test('rejects null', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueString: null }] }]))
+        .toThrow(/valueString must be a string, not null/);
+    });
+
+    test('rejects a number where a string belongs', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueCode: 7 }] }]))
+        .toThrow(/valueCode must be a string/);
+    });
+
+    test('rejects a string where a number belongs', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueInteger: '42' }] }]))
+        .toThrow(/valueInteger must be an integer/);
+    });
+
+    test('rejects a non-integer for an integer', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueInteger: 1.5 }] }]))
+        .toThrow(/valueInteger must be an integer/);
+    });
+
+    test('rejects NaN and Infinity for a decimal', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueDecimal: NaN }] }])).toThrow(/finite/);
+      expect(check([{ name: 'property', part: [{ name: 'value', valueDecimal: Infinity }] }])).toThrow(/finite/);
+    });
+
+    test('rejects a structure inside a valueCoding', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueCoding: { code: { deep: 1 } } }] }]))
+        .toThrow(/valueCoding.code must be a primitive/);
+    });
+
+    test('rejects a valueCoding that is not an object', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueCoding: 'sct' }] }]))
+        .toThrow(/valueCoding must be an object/);
+    });
+
+    test('rejects a value type that has no place in a lookup', () => {
+      expect(check([{ name: 'property', part: [{ name: 'value', valueMeta: { versionId: '1' } }] }]))
+        .toThrow(/valueMeta is not a value type/);
+    });
+
+    test('rejects two values on one parameter', () => {
+      expect(check([{ name: 'property', valueString: 'a', valueCode: 'b' }]))
+        .toThrow(/may carry one value, not 2/);
+    });
+
+    test('rejects a parameter with no name', () => {
+      expect(check([{ valueString: 'a' }])).toThrow(/must have a name/);
+    });
+
+    test('rejects a parameter that is not an object', () => {
+      expect(check(['just a string'])).toThrow(/must be an object/);
+    });
+
+    test('rejects a part that is not an array', () => {
+      expect(check([{ name: 'property', part: { name: 'code' } }])).toThrow(/part must be an array/);
+    });
+  });
+
+  describe('scope', () => {
+    test('only checks what the provider added', () => {
+      const params = [
+        // the worker's own parameters, already built and trusted
+        { name: 'name', valueString: 'Test' },
+        { name: 'suspicious', valueString: { not: 'checked' } }
+      ];
+      expect(check(params, 2)).not.toThrow();
+
+      params.push({ name: 'property', part: [{ name: 'value', valueString: { bad: true } }] });
+      expect(check(params, 2)).toThrow(/valueString must be a string/);
+    });
+
+    test('names the code system, so the log points at the culprit', () => {
+      expect(check([{ name: 'property', valueString: 7 }]))
+        .toThrow(new RegExp(SOURCE.replace(/[/.]/g, '\\$&')));
+    });
+
+    test('is happy with an empty list', () => {
+      expect(check([], 0)).not.toThrow();
+      expect(check([{ name: 'x', valueString: 'y' }], 1)).not.toThrow();
+    });
+  });
+});
diff --git a/tests/tx/xml.test.js b/tests/tx/xml.test.js
index 14bb15fe..9a86139f 100644
--- a/tests/tx/xml.test.js
+++ b/tests/tx/xml.test.js
@@ -23,6 +23,31 @@ describe('FhirXmlBase', () => {
       expect(FhirXmlBase.escapeXml(undefined)).toBe('');
     });
 
+    // XML 1.0 cannot carry the C0 control characters at all - not even as
+    // numeric character references - so serialising one is impossible rather
+    // than merely ugly, and escapeXml refuses instead of emitting a document
+    // no parser will read
+    test('should reject control characters XML cannot represent', () => {
+      for (const code of [0x00, 0x01, 0x08, 0x0B, 0x0C, 0x0E, 0x1F]) {
+        const value = 'before' + String.fromCharCode(code) + 'after';
+        expect(() => FhirXmlBase.escapeXml(value)).toThrow(/control character/);
+      }
+    });
+
+    test('should report which character it rejected, and where', () => {
+      expect(() => FhirXmlBase.escapeXml('ab' + String.fromCharCode(0x0B) + 'cd'))
+        .toThrow(/U\+000B at offset 2/);
+    });
+
+    test('should keep the whitespace XML does allow', () => {
+      const tab = String.fromCharCode(9), lf = String.fromCharCode(10), cr = String.fromCharCode(13);
+      expect(FhirXmlBase.escapeXml('a' + tab + 'b' + lf + 'c' + cr + 'd'))
+        .toBe('a' + tab + 'b' + lf + 'c' + cr + 'd');
+      // DEL is legal in XML 1.0, unlike the C0 range
+      const del = String.fromCharCode(127);
+      expect(FhirXmlBase.escapeXml('a' + del + 'b')).toBe('a' + del + 'b');
+    });
+
     test('should unescape XML entities', () => {
       expect(FhirXmlBase.unescapeXml('a &lt; b')).toBe('a < b');
       expect(FhirXmlBase.unescapeXml('a &gt; b')).toBe('a > b');
diff --git a/tx/library/parameter-check.js b/tx/library/parameter-check.js
new file mode 100644
index 00000000..72a5b165
--- /dev/null
+++ b/tx/library/parameter-check.js
@@ -0,0 +1,139 @@
+//
+// Checking what a code system provider adds to a $lookup response
+//
+// extendLookup() hands the provider the response parameter list and lets it
+// push whatever it likes. Most providers push values they built themselves out
+// of curated content, where the shape is known good. Some pass data through
+// from a remote API, where a value can be any JSON at all - an object, an
+// array, a null - and a structure like that in a FHIR primitive produces a
+// response that is not valid FHIR.
+//
+// That's a bug in the provider, not something the client did, so it fails the
+// operation (500/exception) rather than shipping a malformed response and
+// leaving someone downstream to work out where it came from.
+//
+
+// value[x] elements carried by a JSON string
+const STRING_VALUES = new Set([
+  'valueString', 'valueCode', 'valueUri', 'valueUrl', 'valueCanonical',
+  'valueId', 'valueOid', 'valueUuid', 'valueMarkdown', 'valueBase64Binary',
+  'valueDate', 'valueDateTime', 'valueTime', 'valueInstant'
+]);
+
+// value[x] elements carried by a JSON integer
+const INTEGER_VALUES = new Set(['valueInteger', 'valuePositiveInt', 'valueUnsignedInt']);
+
+// The one complex type a lookup property is allowed to carry. Its own members
+// are all primitives (system, version, code, display, userSelected), so an
+// object or array inside one is just as wrong as it would be at the top.
+const CODING_VALUE = 'valueCoding';
+
+function describe(value) {
+  if (value === null) {
+    return 'null';
+  }
+  if (Array.isArray(value)) {
+    return 'an array';
+  }
+  return 'a ' + typeof value;
+}
+
+function fail(source, path, message) {
+  throw new Error(`Code system provider ${source} produced an invalid $lookup response at ${path}: ${message}`);
+}
+
+function checkCoding(value, source, path) {
+  if (value === null || typeof value !== 'object' || Array.isArray(value)) {
+    fail(source, path, `valueCoding must be an object, not ${describe(value)}`);
+  }
+  for (const [key, member] of Object.entries(value)) {
+    if (member === null || member === undefined) {
+      continue;
+    }
+    const type = typeof member;
+    if (type !== 'string' && type !== 'number' && type !== 'boolean') {
+      fail(source, path, `valueCoding.${key} must be a primitive, not ${describe(member)}`);
+    }
+  }
+}
+
+function checkValue(key, value, source, path) {
+  if (STRING_VALUES.has(key)) {
+    if (typeof value !== 'string') {
+      fail(source, path, `${key} must be a string, not ${describe(value)}`);
+    }
+    return;
+  }
+  if (INTEGER_VALUES.has(key)) {
+    if (!Number.isInteger(value)) {
+      fail(source, path, `${key} must be an integer, not ${describe(value)}`);
+    }
+    return;
+  }
+  if (key === 'valueDecimal') {
+    if (typeof value !== 'number' || !Number.isFinite(value)) {
+      fail(source, path, `${key} must be a finite number, not ${describe(value)}`);
+    }
+    return;
+  }
+  if (key === 'valueBoolean') {
+    if (typeof value !== 'boolean') {
+      fail(source, path, `${key} must be a boolean, not ${describe(value)}`);
+    }
+    return;
+  }
+  if (key === CODING_VALUE) {
+    checkCoding(value, source, path);
+    return;
+  }
+  fail(source, path, `${key} is not a value type a lookup property may carry`);
+}
+
+function checkParameter(param, source, path) {
+  if (param === null || typeof param !== 'object' || Array.isArray(param)) {
+    fail(source, path, `a parameter must be an object, not ${describe(param)}`);
+  }
+  if (typeof param.name !== 'string' || param.name === '') {
+    fail(source, path, `a parameter must have a name, not ${describe(param.name)}`);
+  }
+
+  const name = param.name;
+  const valueKeys = Object.keys(param).filter(key => key.startsWith('value'));
+  if (valueKeys.length > 1) {
+    fail(source, `${path}.${name}`, `a parameter may carry one value, not ${valueKeys.length} (${valueKeys.join(', ')})`);
+  }
+  for (const key of valueKeys) {
+    checkValue(key, param[key], source, `${path}.${name}`);
+  }
+
+  if (param.part !== undefined) {
+    if (!Array.isArray(param.part)) {
+      fail(source, `${path}.${name}`, `part must be an array, not ${describe(param.part)}`);
+    }
+    for (const part of param.part) {
+      checkParameter(part, source, `${path}.${name}`);
+    }
+  }
+}
+
+/**
+ * Check the parameters a provider added to a lookup response.
+ *
+ * @param {Array} params - the whole response parameter list
+ * @param {number} fromIndex - where the provider's own additions start, so
+ *   that only what extendLookup() added is checked
+ * @param {string} source - the code system, for the error message
+ * @throws {Error} if the provider produced something that isn't valid FHIR
+ */
+function checkAddedParameters(params, fromIndex, source) {
+  if (!Array.isArray(params)) {
+    return;
+  }
+  for (let i = fromIndex; i < params.length; i++) {
+    checkParameter(params[i], source || 'unknown', 'parameter');
+  }
+}
+
+module.exports = {
+  checkAddedParameters
+};
diff --git a/tx/workers/lookup.js b/tx/workers/lookup.js
index 6766a0ed..4f456a97 100644
--- a/tx/workers/lookup.js
+++ b/tx/workers/lookup.js
@@ -14,6 +14,7 @@ const {TxParameters} = require("../params");
 const {Parameters} = require("../library/parameters");
 const {Issue, OperationOutcome} = require("../library/operation-outcome");
 const {debugLog} = require("../operation-context");
+const {checkAddedParameters} = require("../library/parameter-check");
 
 class LookupWorker extends TerminologyWorker {
   /**
@@ -393,8 +394,20 @@ class LookupWorker extends TerminologyWorker {
       }
     }
 
-    // Let the provider add additional properties
+    // Let the provider add additional properties. Some providers pass content
+    // through from a remote API, where a value can be any JSON at all, so what
+    // comes back is checked before it goes out: a structure where a FHIR
+    // primitive belongs is a provider bug, and shipping it would produce a
+    // response that isn't valid FHIR (and, in XML, isn't even parseable).
+    const providerParamsFrom = responseParams.length;
     await csProvider.extendLookup(ctxt, params.properties || [], responseParams);
+    let providerName;
+    try {
+      providerName = csProvider.system();
+    } catch (e) {
+      providerName = 'unknown';
+    }
+    checkAddedParameters(responseParams, providerParamsFrom, providerName);
 
     if (reportedSupplements) {
       for (const supplement of reportedSupplements) {
diff --git a/tx/xml/xml-base.js b/tx/xml/xml-base.js
index 49e2c3f5..100a7ea1 100644
--- a/tx/xml/xml-base.js
+++ b/tx/xml/xml-base.js
@@ -486,12 +486,32 @@ class FhirXmlBase {
 
   /**
    * Escape special characters for XML
+   *
+   * XML 1.0 cannot represent the C0 control characters at all - not literally,
+   * and not as numeric character references either - so a value carrying one
+   * cannot be serialised as XML by any means. Tab, line feed and carriage
+   * return are the three exceptions; DEL (U+007F) is legal in XML 1.0 and is
+   * left alone.
+   *
+   * Rather than emit a document no conformant parser will read, this rejects
+   * the value. Anything reaching here with a control character in it came from
+   * outside (a remote API, an imported file) and should have been caught at
+   * the boundary.
+   *
    * @param {*} value - Value to escape
    * @returns {string}
+   * @throws {Error} if the value contains a character XML cannot carry
    */
   static escapeXml(value) {
     if (value === null || value === undefined) return '';
-    return String(value)
+    const str = String(value);
+    // eslint-disable-next-line no-control-regex
+    const illegal = /[\u0000-\u0008\u000B\u000C\u000E-\u001F]/.exec(str);
+    if (illegal) {
+      const code = illegal[0].charCodeAt(0).toString(16).toUpperCase().padStart(4, '0');
+      throw new Error(`Cannot serialise this content as XML: it contains U+${code} at offset ${illegal.index}, a control character XML does not allow`);
+    }
+    return str
       .replace(/&/g, '&amp;')
       .replace(/</g, '&lt;')
       .replace(/>/g, '&gt;')

From bed41db4b36e4613bb72c714a928006633f64088 Mon Sep 17 00:00:00 2001
From: Grahame Grieve <grahameg@gmail.com>
Date: Thu, 27 Aug 2026 12:33:26 +1000
Subject: [PATCH 4/4] set up 0.12.0 release

---
 CHANGELOG.md      | 70 +++++++++++++++++++++++++++++++++++++++++++++++
 package-lock.json |  2 +-
 package.json      |  2 +-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 98f816a9..841b1cde 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,76 @@ All notable changes to the Health Intersections Node Server will be documented i
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.12.0] - 2026-08-27
+
+### Added
+
+- `$cache-control?mode=check`: ask the server whether a cache-id is still alive. The check is
+  also a keepalive - it resets the cache's idle clock - because a client whose own local cache is
+  absorbing the work is invisible to the server, and its cache would otherwise time out mid-job.
+  An unknown id answers 200 with `valid` = false (not 404), so a client can tell "the server says
+  my cache is gone" from "I could not reach the server"; a live cache also reports `sealed`,
+  `resource-count`, `idle` and the server's idle timeout, so a client can work out how often to
+  check instead of guessing
+- Cache tombstones: the server now records why a cache-id it issued stopped existing - closed by
+  the client, expired after N minutes idle, or cleared with everything else - and says which in
+  the error, with the timings. New `CACHE_ID_CLOSED`, `CACHE_ID_EXPIRED` and `CACHE_ID_CLEARED`
+  messages; `CACHE_ID_UNKNOWN` now means "never issued here", and says that a cache-id is only
+  valid on the server instance and endpoint that issued it (#279)
+- SNOMED CT: MRCM validation of postcoordinated expressions - attribute domain (including the
+  lateralizable body structure rule), range (evaluated as ECL, or as a concrete-value range such
+  as `dec(>#0..)`), cardinality and grouping. A constraint that cannot be resolved leaves the
+  value unchecked rather than rejecting it. The MRCM and lateralizable reference sets are now
+  included in the test SNOMED distribution
+- `$translate`: R4 parameter names are accepted as aliases for the R5 ones (`source`/`target` for
+  the scopes, `targetsystem`, `targetcode`), and R4's `reverse` parameter now does literally what
+  it says - swaps the source and target sides once the parameters are read - while remaining an
+  error in R5+, which names the target concept directly instead
+- `$translate`: reverse translation by naming the target concept (`targetCode`, `targetCoding`,
+  `targetCodeableConcept`), with the source system saying which system the answers come from
+- `$translate`: `ConceptMap.group.element.comment` is read from R6 resources and from the
+  cross-version extension on R5 ones, so a preadopted R5 map and a native R6 map behave the same
+- `$translate`: `originMap` names where a chain of maps started, with every other map consulted
+  along the way reported as a `used-conceptmap`; `group.unmapped` is applied per group, so a
+  mapping in one group no longer suppresses another group's fallback
+- Persistent usage statistics: request counts per module, endpoint and operation are written to a
+  SQLite database every `intervalMinutes`, with both the interval count and the all-time total, so
+  they survive restarts and upgrades. New optional `stats` config block (#255)
+- Support for `ValueSet.compose.property` (R6): a value set can name the properties to return in
+  its own expansion, rather than leaving it to the request
+- Publisher: input validation for GitHub owner and repository, git branch, package id and version -
+  enforced on the server, with matching patterns on the form so the browser objects first
+- Publisher: `large-file-archive` config setting - files the IG Publisher leaves in the web output
+  that are too big for GitHub (>100MB) are moved aside for GitHub-hosted websites, instead of
+  leaving the push to fail
+
+### Changed
+
+- `tx/params.js`: parameter names are now interpreted in exactly one place (`seeParameter`), used
+  by both the request `Parameters` resource and the `valueset-expansion-parameter` extension. A
+  parameter from the request always wins over one embedded in a ValueSet's expansion parameters;
+  accumulating parameters (version rules, designations, properties, supplements) still add
+- An inactive display is now governed by `lenient-display-validation` like every other display
+  check - a warning (and `result` = true) when lenient, an error when not - rather than always
+  being a warning. The display is a designation of the concept, just not a current one
+- tx.fhir.org now loads `fhir.tx.support` rather than `fhir.tx.support.r4`
+
+### Fixed
+
+- `$expand`: `status` was lost from imported property declarations, and a concept carrying the
+  same property more than once had the repeats collapsed to a single value
+- Expansion properties are de-duplicated when they arrive from more than one place (the request,
+  an expansion parameter extension, `compose.property`) - a repeat emitted the property twice and
+  changed the cache key
+- `no-cache=true` never busted the cache: the parameter wrote `uid`, which nothing read, instead of
+  the field the cache key hashes
+- Boolean parameters passed as strings (as they always are on a GET) are now accepted, which
+  revives five parameters that were dead on GET requests
+
+### Tx Conformance Statement
+
+FHIRsmith passed all 2822 HL7 terminology service tests (modes tx.fhir.org+omop+general+snomed, tests v1.9.3, runner v6.10.3)
+
 ## [0.11.2] - 2026-08-12
 
 ### Fixed
diff --git a/package-lock.json b/package-lock.json
index 064cb7b9..3b7b5acc 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "fhirsmith",
-  "version": "0.11.2",
+  "version": "0.12.0",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
diff --git a/package.json b/package.json
index 210ed55b..6b626ccf 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "fhirsmith",
-  "version": "0.11.2",
+  "version": "0.12.0",
   "txVersion": "1.9.3",
   "description": "A Node.js server that provides a collection of tools to serve the FHIR ecosystem",
   "main": "server.js",