Document the trash and file versions, which shipped without their pages

backend/src/config/index.js

@@ -1,6 +1,7 @@
 const path = require('path');
 const crypto = require('crypto');
 const env = require('./env');
+const { resolveSessionSecret } = require('./sessionSecret');
 const constants = require('./constants');
 const loggingConfig = require('./logging');
 const { parseByteSize } = require('../utils/env');
@@ -251,7 +252,7 @@ const authMode = determineAuthMode();
 
 const auth = {
   enabled: authMode === 'disabled' ? false : env.AUTH_ENABLED !== false,
-  sessionSecret: env.SESSION_SECRET || crypto.randomBytes(32).toString('hex'),
+  sessionSecret: resolveSessionSecret({ configured: env.SESSION_SECRET, configDir }),
   sessionMaxAgeMs: env.SESSION_MAX_AGE_DAYS * 24 * 60 * 60 * 1000, // Convert days to milliseconds
   mode: authMode,
   oidc: {

backend/src/config/sessionSecret.js

@@ -0,0 +1,135 @@
+const crypto = require('crypto');
+const fs = require('fs');
+const path = require('path');
+const logger = require('../utils/logger');
+
+/**
+ * The secret sessions are signed with, when nobody configured one.
+ *
+ * It was drawn at random at every start. Sessions themselves outlive a restart
+ * — they are kept in CACHE_DIR/sessions.db — but a cookie signed with the
+ * previous secret no longer verifies, so every restart, every upgrade and every
+ * crash signed everyone out. The secrets derived from it (ONLYOFFICE without
+ * ONLYOFFICE_SECRET, the thumbnail links) changed with it.
+ *
+ * So the first start draws one and keeps it in CONFIG_DIR, and later starts read
+ * it back. SESSION_SECRET, when set, always wins and nothing is written.
+ *
+ * Synchronous on purpose: the configuration is required before anything else
+ * runs, and every value derived from the secret is computed at that moment.
+ */
+
+const SECRET_FILE_NAME = 'session-secret';
+const SECRET_PATTERN = /^[0-9a-f]{64}$/i;
+
+const draw = () => crypto.randomBytes(32).toString('hex');
+
+/** Read the stored secret: the secret, `null` when there is none to use, or the error. */
+const readStored = (file) => {
+  let contents;
+  try {
+    contents = fs.readFileSync(file, 'utf8');
+  } catch (error) {
+    if (error.code === 'ENOENT') return { secret: null };
+    return { error };
+  }
+
+  const value = contents.trim();
+  if (SECRET_PATTERN.test(value)) return { secret: value };
+
+  // Nothing of the file's contents is logged: a hand-written secret would
+  // otherwise land in the logs on its way to being replaced.
+  logger.warn(
+    { file, reason: value ? 'not a 64-character hexadecimal secret' : 'empty' },
+    'The stored session secret is unusable and is being replaced; sessions signed with it end ' +
+      'here. To choose the secret yourself, set SESSION_SECRET instead.'
+  );
+  return { secret: null };
+};
+
+/**
+ * Write the secret beside its final name, then rename it into place, so a start
+ * interrupted half-way leaves either no file or a whole one — never a truncated
+ * secret that the next start would have to throw away.
+ *
+ * The staging name is fixed rather than unique: a write that keeps failing (a
+ * full disk) then leaves one stray file that the next attempt reuses, not a new
+ * one per start. Removing it is not this module's business.
+ */
+const store = (configDir, file, secret) => {
+  fs.mkdirSync(configDir, { recursive: true });
+
+  const staging = path.join(configDir, `.${SECRET_FILE_NAME}.tmp`);
+  const fd = fs.openSync(staging, 'w', 0o600);
+  try {
+    // The mode given to open only applies to a file it creates; a staging file
+    // left by an older attempt keeps its own.
+    fs.fchmodSync(fd, 0o600);
+    fs.writeFileSync(fd, `${secret}\n`);
+    fs.fsyncSync(fd);
+  } finally {
+    fs.closeSync(fd);
+  }
+  fs.renameSync(staging, file);
+
+  // The rename only survives a power cut once the directory is on disk too.
+  // Not every platform lets a directory be opened for that, and the file is
+  // already in place, so a refusal here costs nothing but that guarantee.
+  try {
+    const dirFd = fs.openSync(configDir, 'r');
+    try {
+      fs.fsyncSync(dirFd);
+    } finally {
+      fs.closeSync(dirFd);
+    }
+  } catch {
+    /* best effort */
+  }
+};
+
+const warnEphemeral = (configDir, error, action) => {
+  logger.warn(
+    { directory: configDir, code: error.code || null, err: { message: error.message } },
+    `Could not ${action} the session secret in CONFIG_DIR, so a new one is used for this run ` +
+      'only: everyone will be signed out at the next restart. Make CONFIG_DIR writable by the ' +
+      'user the server runs as, or set SESSION_SECRET.'
+  );
+};
+
+/**
+ * @param {object} options
+ * @param {string|null|undefined} options.configured SESSION_SECRET, as read from the environment
+ * @param {string} options.configDir The resolved CONFIG_DIR
+ * @returns {string}
+ */
+const resolveSessionSecret = ({ configured, configDir }) => {
+  if (configured) return configured;
+
+  const file = path.join(configDir, SECRET_FILE_NAME);
+
+  const stored = readStored(file);
+  if (stored.error) {
+    // A file that exists and cannot be read belongs to someone else — another
+    // user, a mount gone wrong. Replacing it would throw away a secret that may
+    // still be good once the permissions are, so it is left alone.
+    warnEphemeral(configDir, stored.error, 'read');
+    return draw();
+  }
+  if (stored.secret) return stored.secret;
+
+  const secret = draw();
+  try {
+    store(configDir, file, secret);
+  } catch (error) {
+    warnEphemeral(configDir, error, 'store');
+    return secret;
+  }
+
+  logger.info(
+    { file },
+    'Generated a session secret and stored it in CONFIG_DIR; sessions now survive restarts'
+  );
+  return secret;
+};
+
+module.exports = { resolveSessionSecret, SECRET_FILE_NAME };

backend/src/middleware/session.js

@@ -1,15 +1,14 @@
-const crypto = require('crypto');
 const session = require('express-session');
 
 const { auth: envAuthConfig } = require('../config/index');
 const { localStore } = require('../utils/sessionStore');
 const logger = require('../utils/logger');
 
 const configureSession = (app) => {
-  const sessionSecret =
-    (envAuthConfig && envAuthConfig.sessionSecret) ||
-    process.env.SESSION_SECRET ||
-    crypto.randomBytes(32).toString('hex');
+  // One source: the configuration resolved it, from SESSION_SECRET or from the
+  // copy kept in CONFIG_DIR. A second fallback drawing its own random secret
+  // here would have signed everyone out whenever it was the one that applied.
+  const sessionSecret = envAuthConfig.sessionSecret;
 
   logger.debug({ hasSessionSecret: Boolean(sessionSecret) }, 'Session secret resolved');
 

backend/src/server.js

@@ -19,6 +19,7 @@ const { purgeExpiredDocumentKeys } = require('./services/onlyofficeDocumentKeySe
 const editorSessions = require('./services/onlyofficeEditorSessionService');
 const { sweepActivity } = require('./services/activityLog');
 const capabilities = require('./services/capabilities');
+const { installProcessFailureHandlers } = require('./utils/processFailures');
 
 let server = null;
 
@@ -124,6 +125,9 @@ const startServer = async () => {
   process.on('SIGTERM', cleanup);
   process.on('SIGINT', cleanup);
 
+  // Installed last, so the shutdown it may need already exists.
+  installProcessFailureHandlers({ onFatal: cleanup });
+
   return server;
 };
 

backend/src/utils/processFailures.js

@@ -0,0 +1,96 @@
+const logger = require('./logger');
+
+/**
+ * What a failure nobody caught should cost.
+ *
+ * Node's default for a rejected promise with no listener is to raise it as an
+ * uncaught exception and stop the process. So one forgotten `await` anywhere in
+ * the application — on a path check, a database read, a stat — answers a single
+ * bad request by taking the server down with it, and everybody else's work goes
+ * with it.
+ *
+ * That is a disproportionate price. A rejection raised while serving a request is
+ * almost always confined to that request: the connection fails, and nothing else
+ * is touched. Reporting it and carrying on is the proportionate answer.
+ *
+ * An uncaught exception is not the same thing and is not treated the same way.
+ * There the stack unwound through code that had no chance to put anything back,
+ * so what is in memory afterwards is unknown — a lock still held, a transaction
+ * half applied. Continuing to serve from that is worse than stopping, so this
+ * stops, deliberately and after saying why.
+ *
+ * None of this hides anything from development. The test suites never load this
+ * file, and the runner already fails a run that leaves an unhandled rejection
+ * behind. The quiet is bought only where the server runs, where staying up is
+ * worth more than dying loudly.
+ */
+
+/** How long a shutdown may take before it is abandoned. */
+const FATAL_SHUTDOWN_TIMEOUT_MS = 5000;
+
+/**
+ * @param {object} [options]
+ * @param {object} [options.log] where to report, injected so a test can read it
+ * @param {() => Promise<void>|void} [options.onFatal] the ordinary shutdown, tried
+ *   before giving up on an uncaught exception
+ * @param {(code: number) => void} [options.exit]
+ * @param {number} [options.shutdownTimeoutMs]
+ * @param {NodeJS.EventEmitter} [options.target] the process to attach to
+ * @returns {() => void} removes both listeners again
+ */
+const installProcessFailureHandlers = ({
+  log = logger,
+  onFatal = null,
+  exit = (code) => process.exit(code),
+  shutdownTimeoutMs = FATAL_SHUTDOWN_TIMEOUT_MS,
+  target = process,
+} = {}) => {
+  const onUnhandledRejection = (reason) => {
+    // Normalised: a rejection carries whatever was thrown, which is often an
+    // Error and sometimes a string nobody meant to reject with.
+    const err = reason instanceof Error ? reason : new Error(String(reason));
+    log.error(
+      { err },
+      'A promise was rejected with nobody listening. The request behind it has failed; the server has not.'
+    );
+  };
+
+  const onUncaughtException = (error) => {
+    log.error(
+      { err: error },
+      'Uncaught exception. Shutting down: what is in memory after this cannot be trusted.'
+    );
+
+    // Bounded, because the shutdown runs in the same unknown state and may never
+    // finish. Whichever comes first wins, and the process ends either way.
+    let ended = false;
+    const end = () => {
+      if (ended) return;
+      ended = true;
+      exit(1);
+    };
+
+    const timer = setTimeout(end, shutdownTimeoutMs);
+    timer.unref?.();
+
+    Promise.resolve()
+      .then(() => onFatal?.())
+      .catch((shutdownError) => {
+        log.error({ err: shutdownError }, 'Shutdown after an uncaught exception failed too');
+      })
+      .finally(() => {
+        clearTimeout(timer);
+        end();
+      });
+  };
+
+  target.on('unhandledRejection', onUnhandledRejection);
+  target.on('uncaughtException', onUncaughtException);
+
+  return () => {
+    target.off('unhandledRejection', onUnhandledRejection);
+    target.off('uncaughtException', onUncaughtException);
+  };
+};
+
+module.exports = { installProcessFailureHandlers, FATAL_SHUTDOWN_TIMEOUT_MS };