Tests pass shell paths through the environment, not into bash -c scripts.

GitHub CodeQL code scanning alerts #549 and #550
(js/shell-command-injection-from-environment): tests/snapSettingsKeys and
tests/snapSocketDenied ran `bash -c` with a script that interpolated an
absolute path built from __dirname (`source "${configPath}"`,
`source "${helper}"`). A checkout path containing a quote, `$(` or a
backtick would change the command that runs. tests/linuxPackageManagers
and tests/snapCaddyNofile6552 had the same shape without an alert yet.

All four now pass the path (and the soft limit, and the wekan-control
function under test) in environment variables, so every script text is
constant. tests/testShellPathsFromEnv.test.cjs fails on any test that
sources an interpolated value, and checks it catches the flagged forms.
These are test files run on a developer machine or CI, so no attack can be
attempted against a running WeKan and nothing is logged in Admin Panel ->
Problems.

Thanks to GitHub CodeQL and xet7 !

tests/linuxPackageManagers.test.cjs

@@ -39,8 +39,8 @@ for (const [name, contents] of Object.entries(fixtures)) {
     const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'wekan-os-release-'));
     const file = path.join(dir, 'os-release');
     fs.writeFileSync(file, contents);
-    const result = spawnSync('bash', ['-c', `. "${helper}"; _et_linux_family`], {
-      encoding: 'utf8', env: { ...process.env, WEKAN_OS_RELEASE_FILE: file },
+    const result = spawnSync('bash', ['-c', '. "$ET_HELPER"; _et_linux_family'], {
+      encoding: 'utf8', env: { ...process.env, ET_HELPER: helper, WEKAN_OS_RELEASE_FILE: file },
     });
     fs.rmSync(dir, { recursive: true, force: true });
     assert.strictEqual(result.status, 0, result.stderr);
@@ -50,8 +50,8 @@ for (const [name, contents] of Object.entries(fixtures)) {
 }
 
 test('macOS is detected without relying on the invoking shell', () => {
-  const result = spawnSync('bash', ['-c', `. "${helper}"; _et_os`], {
-    encoding: 'utf8', env: { ...process.env, WEKAN_UNAME_S: 'Darwin' },
+  const result = spawnSync('bash', ['-c', '. "$ET_HELPER"; _et_os'], {
+    encoding: 'utf8', env: { ...process.env, ET_HELPER: helper, WEKAN_UNAME_S: 'Darwin' },
   });
   assert.strictEqual(result.status, 0, result.stderr);
   assert.strictEqual(result.stdout.trim(), 'macos');

tests/snapCaddyNofile6552.test.cjs

@@ -9,8 +9,8 @@ const { spawnSync } = require('node:child_process');
 const ROOT = path.join(__dirname, '..');
 const helper = path.join(ROOT, 'snap-src/bin/raise-nofile-limit');
 const run = (soft, env = {}) => {
-  const result = spawnSync('bash', ['-c', `ulimit -Sn ${soft} && source "${helper}" && raise_nofile_limit >/dev/null 2>&1; ulimit -Sn; ulimit -Hn`],
-    { env: { PATH: process.env.PATH, ...env }, encoding: 'utf8' });
+  const result = spawnSync('bash', ['-c', 'ulimit -Sn "$SOFT_LIMIT" && source "$NOFILE_HELPER" && raise_nofile_limit >/dev/null 2>&1; ulimit -Sn; ulimit -Hn'],
+    { env: { PATH: process.env.PATH, SOFT_LIMIT: String(soft), NOFILE_HELPER: helper, ...env }, encoding: 'utf8' });
   assert.equal(result.status, 0, result.stderr);
   const [after, hard] = result.stdout.trim().split('\n');
   return { after, hard };

tests/snapSettingsKeys.test.cjs

@@ -28,12 +28,14 @@ function test(name, fn) {
 }
 
 function readConfig(configPath) {
+  // The path goes in through the environment, never into the script text
+  // (CodeQL js/shell-command-injection-from-environment).
   const r = spawnSync('bash', ['-c', `
-    source "${configPath}"
+    source "$SNAP_CONFIG_PATH"
     for k in $keys; do
       kv="KEY_$k"; dv="DEFAULT_$k"; sv="DESCRIPTION_$k"
       printf '%s\\t%s\\t%s\\t%s\\n' "$k" "\${!kv}" "\${!dv+set}" "\${!sv+set}"
-    done`], { encoding: 'utf8' });
+    done`], { encoding: 'utf8', env: { ...process.env, SNAP_CONFIG_PATH: configPath } });
   assert.strictEqual(r.status, 0, r.stderr);
   return r.stdout.trim().split('\n').map((line) => {
     const [name, key, hasDefault, hasDescription] = line.split('\t');

tests/snapSocketDenied.test.cjs

@@ -36,12 +36,14 @@ function test(name, fn) {
 function bash(script, env = {}) {
   return spawnSync('bash', ['-c', script], {
     encoding: 'utf8',
-    env: { ...process.env, SNAP_INSTANCE_NAME: 'wekan', ...env },
+    // The helper's path goes in through the environment, never into the
+    // script text (CodeQL js/shell-command-injection-from-environment).
+    env: { ...process.env, SNAP_INSTANCE_NAME: 'wekan', SOCKET_DENIED_HELPER: helper, ...env },
   });
 }
 
 function denied(err) {
-  return bash(`source "${helper}"; socket_denied "$ERR"`, { ERR: err }).status === 0;
+  return bash(`source "$SOCKET_DENIED_HELPER"; socket_denied "$ERR"`, { ERR: err }).status === 0;
 }
 
 // The exact errors the reporter posted: mongorestore (Go driver) and db-eval
@@ -69,7 +71,7 @@ test('negative: a database that is merely not up is not called an AppArmor probl
 });
 
 test('the explanation names the denial, the upstream bug, and both ways out', () => {
-  const r = bash(`source "${helper}"; socket_denied_hint`);
+  const r = bash(`source "$SOCKET_DENIED_HELPER"; socket_denied_hint`);
   assert.strictEqual(r.status, 0, r.stderr);
   assert.match(r.stdout, /journalctl -k \| grep -i denied/);
   assert.match(r.stdout, /operation="file_perm" class="net"/);
@@ -80,7 +82,7 @@ test('the explanation names the denial, the upstream bug, and both ways out', ()
   assert.match(r.stdout, /sudo snap revert snapd/);
   assert.match(r.stdout, /sudo snap restart wekan/);
   // A parallel install (wekan_2) is told to restart ITSELF.
-  const r2 = bash(`source "${helper}"; socket_denied_hint`, { SNAP_INSTANCE_NAME: 'wekan_2' });
+  const r2 = bash(`source "$SOCKET_DENIED_HELPER"; socket_denied_hint`, { SNAP_INSTANCE_NAME: 'wekan_2' });
   assert.match(r2.stdout, /sudo snap restart wekan_2/);
 });
 
@@ -112,8 +114,8 @@ test('show_socket_denied prints only for a refusal, and says so in the log', ()
   const start = control.indexOf('show_socket_denied() {');
   const fn = control.slice(start, control.indexOf('\n}\n', start) + 3);
   const run = (err) => bash(
-    `source "${helper}"; MONGO_HOST=127.0.0.1; MONGO_PORT=27019\n${fn}\nshow_socket_denied FerretDB "$ERR"`,
-    { ERR: err },
+    'source "$SOCKET_DENIED_HELPER"; MONGO_HOST=127.0.0.1; MONGO_PORT=27019; eval "$SHOW_FN"; show_socket_denied FerretDB "$ERR"',
+    { ERR: err, SHOW_FN: fn },
   );
   const yes = run(REPORTED[0]);
   assert.strictEqual(yes.status, 0);

tests/testShellPathsFromEnv.test.cjs

@@ -0,0 +1,29 @@
+'use strict';
+// CodeQL js/shell-command-injection-from-environment (alerts #549, #550): tests
+// ran `bash -c` with a script that interpolated an absolute path built from
+// __dirname, e.g. `source "${helper}"`. A checkout path with a quote, `$(` or a
+// backtick would change the command. The path goes in through the environment
+// instead (`source "$HELPER"`), so the script text is constant.
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const root = path.resolve(__dirname, '..');
+// A shell `source`/`.` of an interpolated JavaScript value inside a template.
+const SHAPE = /(?:^|[\s;&|(`'"])(?:source|\.)\s+"?\$\{[^}]+\}/m;
+
+const offenders = fs.readdirSync(path.join(root, 'tests'))
+  .filter(name => /\.(c?js|mjs)$/.test(name) && name !== path.basename(__filename))
+  .filter(name => SHAPE.test(fs.readFileSync(path.join(root, 'tests', name), 'utf8')))
+  .sort();
+assert.deepEqual(offenders, [], 'pass the path in an environment variable: source "$HELPER"');
+
+// Negative: the guard does catch the shapes that were flagged.
+assert.ok(SHAPE.test('spawnSync(\'bash\', [\'-c\', `source "${helper}"; socket_denied`])'));
+assert.ok(SHAPE.test('spawnSync(\'bash\', [\'-c\', `. "${helper}"; _et_os`])'));
+assert.ok(SHAPE.test('`\n    source "${configPath}"\n`'));
+// ...and not the fixed form, nor a JavaScript template that merely mentions a dot.
+assert.ok(!SHAPE.test('[\'-c\', \'source "$SOCKET_DENIED_HELPER"; socket_denied\']'));
+assert.ok(!SHAPE.test('`${name}. ${detail}`'));
+
+console.log('testShellPathsFromEnv: no test sources an interpolated path');