Security: stop the container cleanly and harden its runtime

Dockerfile

@@ -60,6 +60,7 @@ RUN echo "packages upgraded for ${APP_VERSION}" && \
 COPY nginx/dashboard.conf /etc/nginx/http.d/dashboard.conf
 COPY nginx/security-headers.conf /etc/nginx/http.d/security-headers.conf
 COPY nginx/csp-default.conf /etc/nginx/http.d/csp-default.conf
+COPY nginx/csp-api.conf /etc/nginx/http.d/csp-api.conf
 # Replaced at container start by docker-entrypoint.sh. Present so the config is
 # valid at build time.
 COPY nginx/realip.conf /etc/nginx/http.d/realip.conf
@@ -68,11 +69,11 @@ COPY --from=assets /src/ui/ /usr/share/nginx/html/
 
 # The image mirrors the repository layout. Shared modules keep the same relative
 # path in both places.
-COPY --chown=node:node api/ /app/api/
-COPY --chown=node:node ui/js/link-url.js /app/ui/js/link-url.js
-COPY --chown=node:node ui/js/limits.js /app/ui/js/limits.js
-COPY --chown=node:node ui/js/config-recovery-logic.js /app/ui/js/config-recovery-logic.js
-COPY --chown=node:node ui/js/host-names.js /app/ui/js/host-names.js
+COPY api/ /app/api/
+COPY ui/js/link-url.js /app/ui/js/link-url.js
+COPY ui/js/limits.js /app/ui/js/limits.js
+COPY ui/js/config-recovery-logic.js /app/ui/js/config-recovery-logic.js
+COPY ui/js/host-names.js /app/ui/js/host-names.js
 COPY scripts/exit-on-fatal.py /app/scripts/exit-on-fatal.py
 # Fails the build if python3 is no longer present. Without it the event listener
 # cannot start and no failure is reported.

api/test/link-url.test.js

@@ -114,8 +114,8 @@ test('the Dockerfile puts the shared rule where the server can require it', () =
   const dockerfile = fs.readFileSync(path.join(__dirname, '../../Dockerfile'), 'utf8');
   /* The image mirrors the repository layout, so the same relative path resolves
      in both. If either line changes without the other, the API cannot start. */
-  assert.match(dockerfile, /COPY --chown=node:node api\/ \/app\/api\//);
-  assert.match(dockerfile, /COPY --chown=node:node ui\/js\/link-url\.js \/app\/ui\/js\/link-url\.js/);
+  assert.match(dockerfile, /^COPY api\/ \/app\/api\/$/m);
+  assert.match(dockerfile, /^COPY ui\/js\/link-url\.js \/app\/ui\/js\/link-url\.js$/m);
   const supervisord = fs.readFileSync(path.join(__dirname, '../../supervisord.conf'), 'utf8');
   assert.match(supervisord, /command=node \/app\/api\/src\/server\.js/);
 });

api/test/nginx-config.test.js

@@ -462,3 +462,23 @@ test('the image stamps assets itself, so a locally built image is not pinned to
     'the web root must come from the stamped stage, not straight from the build context',
   );
 });
+
+/* ── the API policy ───────────────────────────────────────────────────────── */
+
+/* /api/ sets its own headers, so the server-level policy does not reach it. The
+   icon proxy serves third-party SVG on this origin. */
+test('every /api/ location sends the locked-down API policy', () => {
+  const apiPolicy = (read('csp-api.conf').match(/add_header Content-Security-Policy "([^"]+)"/) || [])[1];
+  assert.ok(apiPolicy, 'csp-api.conf has no policy');
+  for (const directive of ["default-src 'none'", "frame-ancestors 'none'", 'sandbox']) {
+    assert.ok(apiPolicy.includes(directive), `the API policy lacks ${directive}`);
+  }
+
+  const locations = [...dashboard.matchAll(/location (?:= )?(\/api\/\S*) \{/g)];
+  assert.equal(locations.length, 3, 'an /api/ location was added or removed; check it sends the API policy');
+  for (const m of locations) {
+    const block = dashboard.slice(m.index, dashboard.indexOf('\n    }', m.index));
+    assert.ok(block.includes('include /etc/nginx/http.d/csp-api.conf;'), `${m[1]} sends no API policy`);
+    assert.ok(!block.includes(INCLUDE), `${m[1]} also sends the page policy`);
+  }
+});

docker-compose.yml

@@ -13,6 +13,7 @@ services:
       - SETUID
       - SETGID
       - DAC_OVERRIDE
+      - KILL
     security_opt:
       - no-new-privileges:true
     privileged: false

docker-entrypoint.sh

@@ -77,7 +77,7 @@ nginx -t -q
 # marker carries that failure out as the container's exit code, so a dead API
 # reads as a failure rather than a normal shutdown and `restart: on-failure`
 # behaves the same as `unless-stopped` here.
-MARKER="${SUPERVISOR_FATAL_MARKER:-/tmp/stackyard-fatal}"
+MARKER="${SUPERVISOR_FATAL_MARKER:-/run/stackyard-fatal}"
 rm -f "$MARKER"
 
 # Run in the background rather than exec, so this script survives to read the
@@ -88,13 +88,14 @@ rm -f "$MARKER"
 child=$!
 trap 'kill -TERM "$child" 2>/dev/null' TERM INT
 
-# `wait` returns early when a signal is handled, so wait again for the real exit.
-wait "$child"
-status=$?
-if [ "$status" -gt 128 ]; then
-  wait "$child" 2>/dev/null
-  status=$?
-fi
+# Keep `|| status=$?`. A trapped signal makes `wait` return 143, and set -e
+# would exit here: the kernel then kills supervisord mid-shutdown.
+status=0
+wait "$child" || status=$?
+while kill -0 "$child" 2>/dev/null; do
+  status=0
+  wait "$child" || status=$?
+done
 
 if [ -f "$MARKER" ]; then
   echo "stackyard: $(cat "$MARKER") could not be started; exiting so the container is restarted" >&2

nginx/csp-api.conf

@@ -0,0 +1,3 @@
+# Policy for every /api/ location. Do not loosen it. A proxied SVG icon opened
+# directly would run script on this origin.
+add_header Content-Security-Policy "default-src 'none'; frame-ancestors 'none'; sandbox" always;

nginx/dashboard.conf

@@ -162,6 +162,7 @@ server {
         proxy_set_header X-Real-IP $remote_addr;
         proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
         include /etc/nginx/http.d/security-headers.conf;
+        include /etc/nginx/http.d/csp-api.conf;
         add_header Cache-Control "no-store" always;
         proxy_read_timeout    60s;
         proxy_connect_timeout 5s;
@@ -179,6 +180,7 @@ server {
         proxy_set_header X-Real-IP $remote_addr;
         proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
         include /etc/nginx/http.d/security-headers.conf;
+        include /etc/nginx/http.d/csp-api.conf;
         add_header Cache-Control "no-store" always;
         proxy_read_timeout    60s;
         proxy_connect_timeout 5s;
@@ -199,6 +201,7 @@ server {
         # $scheme is always http here, and setting it would overwrite the https a
         # front proxy sent and drop the Secure flag from the session cookie.
         include /etc/nginx/http.d/security-headers.conf;
+        include /etc/nginx/http.d/csp-api.conf;
         add_header Cache-Control "no-store" always;
         # Give up after 15s and return 504 if Node hangs.
         proxy_read_timeout    15s;

nginx/security-headers.conf

@@ -1,7 +1,7 @@
 # Server-level security headers. nginx add_header does not inherit into a
 # location that sets any add_header of its own, so every such location includes
 # this file. X-Frame-Options is set per location. Content-Security-Policy comes
-# from csp-default.conf, except in /admin and /widgets/.
+# from csp-default.conf, except in /admin, /widgets/ and /api/ (csp-api.conf).
 add_header X-Content-Type-Options "nosniff" always;
 add_header Referrer-Policy "strict-origin-when-cross-origin" always;
 add_header Permissions-Policy "camera=(), microphone=(), geolocation=()" always;

scripts/exit-on-fatal.py

@@ -24,7 +24,7 @@
 import sys
 
 # Read by docker-entrypoint.sh after supervisord exits.
-FAILURE_MARKER = os.environ.get('SUPERVISOR_FATAL_MARKER', '/tmp/stackyard-fatal')
+FAILURE_MARKER = os.environ.get('SUPERVISOR_FATAL_MARKER', '/run/stackyard-fatal')
 
 
 def write_stdout(s):

supervisord.conf

@@ -15,7 +15,7 @@ logfile_backups=3
 ; this repo prints itself, from scripts/exit-on-fatal.py and
 ; docker-entrypoint.sh. Do not restore info without re-reading that trade.
 loglevel=warn
-pidfile=/tmp/supervisord.pid
+pidfile=/run/supervisord.pid
 
 [program:nginx]
 command=/usr/sbin/nginx -g "daemon off;"

templates/stackyard.xml

@@ -5,6 +5,7 @@
   <Registry>https://ghcr.io</Registry>
   <Network>bridge</Network>
   <Privileged>false</Privileged>
+  <ExtraParams>--init --cap-drop=ALL --cap-add=NET_BIND_SERVICE --cap-add=CHOWN --cap-add=SETUID --cap-add=SETGID --cap-add=DAC_OVERRIDE --cap-add=KILL --security-opt=no-new-privileges:true --pids-limit=256 --memory=384m --memory-reservation=96m --memory-swap=384m --cpus=0.5</ExtraParams>
   <Support>https://github.com/SandObserver/Stackyard/issues</Support>
   <Project>https://github.com/SandObserver/Stackyard</Project>
   <ReadMe>https://github.com/SandObserver/Stackyard/blob/main/README.md</ReadMe>
@@ -79,4 +80,5 @@
   <Config Name="Allow Private IPs" Target="ALLOW_PRIVATE_IPS" Default="true" Mode="" Description="Let widgets and badges reach private addresses, which most homelab services use. Set to false to keep the SSRF guard on." Type="Variable" Display="always" Required="false" Mask="false">true</Config>
   <Config Name="Docker Socket Proxy URL" Target="SOCKET_PROXY_URL" Default="" Mode="" Description="Optional. Address of a Docker socket proxy, for the Docker widgets. Example: http://socket-proxy:2375" Type="Variable" Display="advanced" Required="false" Mask="false"></Config>
   <Config Name="Trust Proxy" Target="TRUST_PROXY" Default="" Mode="" Description="Optional. Set to true when running behind a reverse proxy, so client addresses are read from the forwarded headers." Type="Variable" Display="advanced" Required="false" Mask="false"></Config>
+  <Config Name="Node Options" Target="NODE_OPTIONS" Default="--max-old-space-size=192" Mode="" Description="Node heap cap. Keep it near half the --memory limit in Extra Parameters, and raise both together." Type="Variable" Display="advanced" Required="false" Mask="false">--max-old-space-size=192</Config>
 </Container>

test/repo/ca-template.test.js

@@ -118,3 +118,41 @@ test('the donation link is the canonical form', () => {
     assert.ok(tag(xml, 'DonateText'));
   }
 });
+
+/* An Unraid install gets no other hardening: the Compose file does not apply. */
+test('the template applies the Compose hardening', () => {
+  const compose = fs.readFileSync(path.join(ROOT, 'docker-compose.yml'), 'utf8');
+  const value = key => {
+    const m = new RegExp(`^\\s+${key}:\\s*"?([^"\\s]+)"?$`, 'm').exec(compose);
+    assert.ok(m, `docker-compose.yml has no ${key}`);
+    return m[1];
+  };
+  const list = key => {
+    const m = new RegExp(`^\\s+${key}:\\n((?:\\s+- .+\\n)+)`, 'm').exec(compose);
+    assert.ok(m, `docker-compose.yml has no ${key}`);
+    return [...m[1].matchAll(/- (\S+)/g)].map(i => i[1]);
+  };
+
+  const expected = [
+    ...(value('init') === 'true' ? ['--init'] : []),
+    ...list('cap_drop').map(c => `--cap-drop=${c}`),
+    ...list('cap_add').map(c => `--cap-add=${c}`),
+    ...list('security_opt').map(o => `--security-opt=${o}`),
+    `--pids-limit=${value('pids_limit')}`,
+    `--memory=${value('mem_limit')}`,
+    `--memory-reservation=${value('mem_reservation')}`,
+    `--memory-swap=${value('memswap_limit')}`,
+    `--cpus=${value('cpus')}`,
+  ];
+  const params = (tag(template, 'ExtraParams') || '').split(/\s+/);
+  assert.deepEqual(params.toSorted(), expected.toSorted());
+
+  /* Node sizes its heap from the host, not --memory. Without the cap the kernel
+     kills the API instead of it collecting. */
+  const heap = /NODE_OPTIONS=\$\{NODE_OPTIONS:-([^}]+)\}/.exec(compose);
+  assert.ok(heap, 'docker-compose.yml has no NODE_OPTIONS default');
+  const config = /<Config[^>]*Target="NODE_OPTIONS"[^>]*Default="([^"]+)"[^>]*>([^<]*)<\/Config>/.exec(template);
+  assert.ok(config, 'the template sets a memory limit but no heap cap');
+  assert.equal(config[1], heap[1]);
+  assert.equal(config[2], heap[1]);
+});

test/repo/compose-settings.test.js

@@ -70,3 +70,9 @@ test('the Compose hardening is in place', () => {
   assert.ok(!/^\s*user:/m.test(compose), 'the Compose file now sets a user');
   assert.match(read('supervisord.conf'), /^user=node$/m, 'the API should run as node');
 });
+
+/* supervisord runs as root and the API as node. Without KILL, root cannot signal
+   another user's process, so a stop never reaches the API. */
+test('supervisord can stop the API under the Compose capabilities', () => {
+  assert.match(compose, /cap_add:\s*\n(?:\s*- \w+\n)*\s*- KILL\n/);
+});

test/repo/entrypoint-shutdown.test.js

@@ -0,0 +1,110 @@
+/* `docker stop` sends SIGTERM to the entrypoint, which forwards it to
+   supervisord. The entrypoint must outlive supervisord's shutdown: once PID 1
+   exits, the kernel kills every process left in the container.
+
+   Runs the real entrypoint with a stand-in nginx and a stand-in supervisord
+   that takes a moment to shut down. */
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { spawn } = require('node:child_process');
+
+const { tmpDir } = require('../../api/test-support/tmp');
+
+const ENTRYPOINT = path.join(__dirname, '../../docker-entrypoint.sh');
+
+const stub = (dir, name, body) => {
+  const p = path.join(dir, name);
+  fs.writeFileSync(p, `#!/bin/sh\n${body}\n`);
+  fs.chmodSync(p, 0o755);
+  return p;
+};
+
+const waitFor = async (check, ms = 5000) => {
+  const end = Date.now() + ms;
+  while (!check()) {
+    if (Date.now() > end) throw new Error('timed out');
+    await new Promise(r => setTimeout(r, 20));
+  }
+};
+
+/* @returns {Promise<{ code: number|null, stopped: boolean, stderr: string }>}
+   `stopped` is whether the stand-in had finished its shutdown when the
+   entrypoint exited. */
+async function run({ child, signals = ['SIGTERM'], marker = false }) {
+  const dir = tmpDir('entrypoint-stop');
+  const binDir = path.join(dir, 'bin');
+  fs.mkdirSync(binDir);
+  stub(binDir, 'nginx', 'exit 0');
+  const handoff = stub(binDir, 'handoff', child);
+  const started = path.join(dir, 'started');
+  const done = path.join(dir, 'done');
+  const fatal = path.join(dir, 'fatal');
+
+  const proc = spawn('sh', [ENTRYPOINT, handoff], {
+    env: {
+      PATH: `${binDir}:${process.env.PATH}`,
+      REALIP_CONF: path.join(dir, 'realip.conf'),
+      LISTEN_CONF: path.join(dir, 'listen-ipv6.inc'),
+      DATA_DIR: path.join(dir, 'no-data'),
+      ICONS_DIR: path.join(dir, 'no-icons'),
+      SUPERVISOR_FATAL_MARKER: fatal,
+      SY_STARTED: started,
+      SY_DONE: done,
+      SY_FATAL: marker ? fatal : '',
+    },
+    stdio: ['ignore', 'ignore', 'pipe'],
+  });
+  let stderr = '';
+  proc.stderr.on('data', d => {
+    stderr += d;
+  });
+  const exited = new Promise(resolve => proc.on('exit', status => resolve(status)));
+
+  await waitFor(() => fs.existsSync(started));
+  for (const [i, sig] of signals.entries()) {
+    if (i) await new Promise(r => setTimeout(r, 100));
+    proc.kill(sig);
+  }
+  const code = await exited;
+  return { code, stopped: fs.existsSync(done), stderr };
+}
+
+/* Shuts down over 300 ms after SIGTERM, then exits 0 as supervisord does. */
+const SLOW_STOP = [
+  'trap \'sleep 0.3; [ -n "$SY_FATAL" ] && echo api > "$SY_FATAL"; touch "$SY_DONE"; exit 0\' TERM',
+  'touch "$SY_STARTED"',
+  'while :; do sleep 0.05; done',
+].join('\n');
+
+test('docker stop waits for supervisord to finish shutting down', async () => {
+  const r = await run({ child: SLOW_STOP });
+  assert.ok(r.stopped, 'the entrypoint exited while supervisord was still shutting down');
+  assert.equal(r.code, 0, 'a clean stop must exit with supervisord status');
+});
+
+test('Ctrl-C in the foreground waits the same way', async () => {
+  const r = await run({ child: SLOW_STOP, signals: ['SIGINT'] });
+  assert.ok(r.stopped);
+  assert.equal(r.code, 0);
+});
+
+test('a second signal during shutdown still waits', async () => {
+  const r = await run({ child: SLOW_STOP, signals: ['SIGTERM', 'SIGTERM'] });
+  assert.ok(r.stopped, 'the entrypoint exited on the second signal');
+  assert.equal(r.code, 0);
+});
+
+test('a program that could not be started still fails the container after a stop', async () => {
+  const r = await run({ child: SLOW_STOP, marker: true });
+  assert.ok(r.stopped);
+  assert.equal(r.code, 1);
+  assert.match(r.stderr, /api could not be started/);
+});
+
+test('supervisord exiting on its own passes its status through', async () => {
+  const r = await run({ child: 'touch "$SY_STARTED"; sleep 0.1; exit 3', signals: [] });
+  assert.equal(r.code, 3);
+});

test/repo/image-hardening.test.js

@@ -119,3 +119,12 @@ test('the upgrade cannot be served from a cache across releases', () => {
   const argAt = dockerfile.lastIndexOf('ARG APP_VERSION', dockerfile.indexOf(run));
   assert.ok(argAt !== -1, 'APP_VERSION is read in the step but never declared above it');
 });
+
+/* The API runs as node. Code it owns, it can rewrite, and the change survives a
+   restart. */
+test('the API cannot rewrite its own code', () => {
+  const runtime = dockerfile.split(/^FROM /m).pop();
+  const copies = runtime.split('\n').filter(l => /^COPY .*\/app\//.test(l));
+  assert.ok(copies.length > 0);
+  for (const line of copies) assert.doesNotMatch(line, /--chown/, line);
+});

test/repo/supervisord-fatal.test.js

@@ -182,3 +182,13 @@ test('only the supervisor listener reaches the runtime image', () => {
   const copied = [...runtime.matchAll(/^COPY (?:--\S+ )*(scripts\/\S+)/gm)].map(m => m[1]);
   assert.deepEqual(copied, ['scripts/exit-on-fatal.py']);
 });
+
+/* supervisord and the listener run as root, and the API user can write /tmp. A
+   link planted there would make root overwrite the file it points at. */
+test('root writes its marker and pid file outside world-writable /tmp', () => {
+  const marker = /SUPERVISOR_FATAL_MARKER',\s*'([^']+)'/.exec(listener)[1];
+  const pidfile = /^pidfile=(\S+)$/m.exec(supervisord)[1];
+  for (const file of [marker, pidfile]) {
+    assert.match(file, /^\/run\//, `${file} is not in a root-owned directory`);
+  }
+});