feat(server): add startup security checks for secrets and origins

.changeset/security-startup-checks.md

@@ -0,0 +1,7 @@
+---
+---
+
+Add server startup security checks for session secrets and allowed origins.
+Not exported from @chatcops/server yet; the checks will be called by the
+handler in a follow-up PR. Empty changeset per issue requirement.
+---

packages/server/src/security/startup-checks.ts

@@ -0,0 +1,141 @@
+/**
+ * Startup checks for the security settings the server is about to add: the
+ * allowed origins and the secrets that sign session tokens.
+ *
+ * These are the mistakes that fail silently. A secret that is ten characters
+ * long works, and so does a secret that is also a `NEXT_PUBLIC_` variable, so
+ * nothing breaks until the day it does. A function that finds them before the
+ * server starts is worth more than one that finds them during a request.
+ *
+ * Nothing here logs or throws. The caller decides what to do with a problem.
+ */
+
+/** The security settings a server is configured with. */
+export interface SecuritySettings {
+  /** The origins the server accepts requests from. `*` allows every origin. */
+  allowedOrigins?: string[];
+  /** The secrets that sign session tokens. */
+  sessionSecrets?: string[];
+}
+
+/** One thing that is wrong with the security settings. */
+export interface SecurityProblem {
+  /** An error stops the server from starting. A warning is worth a look. */
+  level: 'error' | 'warning';
+  /** What is wrong, and what to do about it. Never contains a secret. */
+  message: string;
+}
+
+/** How many bytes a session secret needs to be worth signing with. */
+const MIN_SECRET_BYTES = 32;
+
+/**
+ * The env var prefixes whose value is shipped to the browser, so a secret
+ * that shares a value with one of them is in the client bundle.
+ */
+const PUBLIC_ENV_PREFIXES = ['NEXT_PUBLIC_', 'VITE_', 'PUBLIC_', 'NUXT_PUBLIC_', 'EXPO_PUBLIC_', 'REACT_APP_'];
+
+/**
+ * The environment to read, which is `process.env` where there is one. Workers
+ * have no `process`, so the checks still work there with an empty environment.
+ */
+function defaultEnv(): Record<string, string | undefined> {
+  return (globalThis as { process?: { env?: Record<string, string | undefined> } }).process?.env ?? {};
+}
+
+/** How many bytes a string takes in UTF-8, so a 32 byte secret is 32 bytes. */
+function utf8Length(value: string): number {
+  return new TextEncoder().encode(value).length;
+}
+
+/**
+ * Finds the mistakes in the security settings, most serious first.
+ *
+ * `env` is the environment to look in, and defaults to `process.env`.
+ */
+export function checkSecuritySettings(
+  settings: SecuritySettings,
+  env?: Record<string, string | undefined>,
+): SecurityProblem[] {
+  const problems: SecurityProblem[] = [];
+  const secrets = settings.sessionSecrets ?? [];
+  const origins = settings.allowedOrigins ?? [];
+  const environment = env ?? defaultEnv();
+
+  for (const [index, secret] of secrets.entries()) {
+    if (utf8Length(secret) < MIN_SECRET_BYTES) {
+      problems.push({
+        level: 'error',
+        message: `Session secret ${index + 1} is shorter than ${MIN_SECRET_BYTES} bytes.`,
+      });
+    }
+  }
+
+  for (const [index, secret] of secrets.entries()) {
+    for (const name of Object.keys(environment)) {
+      if (!PUBLIC_ENV_PREFIXES.some((prefix) => name.startsWith(prefix))) continue;
+      if (environment[name] !== secret) continue;
+      problems.push({
+        level: 'error',
+        message:
+          `Session secret ${index + 1} is also the value of the ${name} environment variable, ` +
+          'which is sent to the browser. Use a different secret.',
+      });
+    }
+  }
+
+  for (const [index, secret] of secrets.entries()) {
+    const first = secrets.indexOf(secret);
+    if (first !== index) {
+      problems.push({
+        level: 'warning',
+        message: `Session secrets ${first + 1} and ${index + 1} are the same.`,
+      });
+    }
+  }
+
+  for (const [index, origin] of origins.entries()) {
+    if (origin === '*') continue;
+    if (!origin.startsWith('http://') && !origin.startsWith('https://')) {
+      problems.push({
+        level: 'error',
+        message: `Allowed origin ${index + 1} must start with http:// or https://.`,
+      });
+      continue;
+    }
+    let parsed: URL;
+    try {
+      parsed = new URL(origin);
+    } catch {
+      problems.push({
+        level: 'error',
+        message: `Allowed origin ${index + 1} is not a valid URL.`,
+      });
+      continue;
+    }
+    if (parsed.pathname !== '/' || parsed.search !== '' || origin.endsWith('/')) {
+      problems.push({
+        level: 'error',
+        message:
+          `Allowed origin ${index + 1} must not have a path, a query or a trailing slash, ` +
+          'because browsers send the origin without one.',
+      });
+      continue;
+    }
+    if (origin.startsWith('http://') && parsed.hostname !== 'localhost' && parsed.hostname !== '127.0.0.1') {
+      problems.push({
+        level: 'warning',
+        message: `Allowed origin ${index + 1} uses http://. Use https://, unless it is a local development server.`,
+      });
+    }
+  }
+
+  if (origins.includes('*') && origins.length > 1) {
+    problems.push({
+      level: 'warning',
+      message: `Allowed origins include "*", so the other ${origins.length - 1} entries do nothing.`,
+    });
+  }
+
+  return problems;
+}

packages/server/tests/security/startup-checks.test.ts

@@ -0,0 +1,151 @@
+import { describe, it, expect } from 'vitest';
+import { checkSecuritySettings } from '../../src/security/startup-checks.js';
+
+const LONG_SECRET = 'a-secret-that-is-at-least-32-bytes-long';
+
+describe('checkSecuritySettings', () => {
+  it('says nothing about a clean configuration', () => {
+    const problems = checkSecuritySettings(
+      { allowedOrigins: ['https://acme.com'], sessionSecrets: [LONG_SECRET] },
+      {},
+    );
+    expect(problems).toEqual([]);
+  });
+
+  it('says nothing when there is nothing to check', () => {
+    expect(checkSecuritySettings({}, {})).toEqual([]);
+  });
+
+  it('reports a session secret shorter than 32 bytes', () => {
+    const problems = checkSecuritySettings({ sessionSecrets: ['short'] }, {});
+    expect(problems).toEqual([
+      { level: 'error', message: 'Session secret 1 is shorter than 32 bytes.' },
+    ]);
+  });
+
+  it('counts the bytes of a secret, not its characters', () => {
+    // 16 two-byte characters are 32 bytes, so this one is long enough.
+    const problems = checkSecuritySettings({ sessionSecrets: ['é'.repeat(16)] }, {});
+    expect(problems).toEqual([]);
+  });
+
+  it('reports a secret that is also a public environment variable', () => {
+    const problems = checkSecuritySettings(
+      { sessionSecrets: [LONG_SECRET] },
+      { NEXT_PUBLIC_SESSION_SECRET: LONG_SECRET },
+    );
+    expect(problems).toEqual([
+      {
+        level: 'error',
+        message:
+          'Session secret 1 is also the value of the NEXT_PUBLIC_SESSION_SECRET environment variable, ' +
+          'which is sent to the browser. Use a different secret.',
+      },
+    ]);
+  });
+
+  it('reports a secret for every public environment variable prefix', () => {
+    for (const prefix of ['NEXT_PUBLIC_', 'VITE_', 'PUBLIC_', 'NUXT_PUBLIC_', 'EXPO_PUBLIC_', 'REACT_APP_']) {
+      const problems = checkSecuritySettings(
+        { sessionSecrets: [LONG_SECRET] },
+        { [`${prefix}KEY`]: LONG_SECRET },
+      );
+      expect(problems).toHaveLength(1);
+      expect(problems[0].message).toContain(`${prefix}KEY`);
+    }
+  });
+
+  it('ignores a private environment variable with the same value', () => {
+    const problems = checkSecuritySettings(
+      { sessionSecrets: [LONG_SECRET] },
+      { SESSION_SECRET: LONG_SECRET },
+    );
+    expect(problems).toEqual([]);
+  });
+
+  it('never puts the secret in a message', () => {
+    const secret = 'thirty-two-bytes-exactly-long-ok';
+    const problems = checkSecuritySettings(
+      { sessionSecrets: [secret] },
+      { NEXT_PUBLIC_KEY: secret },
+    );
+    expect(problems.length).toBeGreaterThan(0);
+    for (const problem of problems) {
+      expect(problem.message).not.toContain(secret);
+    }
+  });
+
+  it('reports two session secrets that are the same', () => {
+    const problems = checkSecuritySettings({ sessionSecrets: [LONG_SECRET, LONG_SECRET] }, {});
+    expect(problems).toEqual([
+      { level: 'warning', message: 'Session secrets 1 and 2 are the same.' },
+    ]);
+  });
+
+  it('reports an allowed origin that is not a URL', () => {
+    const problems = checkSecuritySettings({ allowedOrigins: ['acme.com'] }, {});
+    expect(problems).toEqual([
+      { level: 'error', message: 'Allowed origin 1 must start with http:// or https://.' },
+    ]);
+  });
+
+  it('reports an allowed origin that is not a valid URL', () => {
+    const problems = checkSecuritySettings({ allowedOrigins: ['https://'] }, {});
+    expect(problems).toEqual([
+      { level: 'error', message: 'Allowed origin 1 is not a valid URL.' },
+    ]);
+  });
+
+  it('reports an allowed origin with a path, a query or a trailing slash', () => {
+    const problems = checkSecuritySettings(
+      { allowedOrigins: ['https://acme.com/app', 'https://acme.com/?a=1', 'https://acme.com/'] },
+      {},
+    );
+    expect(problems).toHaveLength(3);
+    for (const problem of problems) {
+      expect(problem.level).toBe('error');
+      expect(problem.message).toContain('must not have a path, a query or a trailing slash');
+    }
+  });
+
+  it('allows a plain http:// origin on localhost', () => {
+    const problems = checkSecuritySettings(
+      { allowedOrigins: ['http://localhost:5173', 'http://127.0.0.1:8080'] },
+      {},
+    );
+    expect(problems).toEqual([]);
+  });
+
+  it('warns about an http:// origin that is not local', () => {
+    const problems = checkSecuritySettings({ allowedOrigins: ['http://acme.com'] }, {});
+    expect(problems).toEqual([
+      {
+        level: 'warning',
+        message: 'Allowed origin 1 uses http://. Use https://, unless it is a local development server.',
+      },
+    ]);
+  });
+
+  it('warns when "*" is used together with other origins', () => {
+    const problems = checkSecuritySettings({ allowedOrigins: ['*', 'https://acme.com'] }, {});
+    expect(problems).toEqual([
+      { level: 'warning', message: 'Allowed origins include "*", so the other 1 entries do nothing.' },
+    ]);
+  });
+
+  it('allows "*" on its own', () => {
+    expect(checkSecuritySettings({ allowedOrigins: ['*'] }, {})).toEqual([]);
+  });
+
+  it('reads the environment from process.env when none is given', () => {
+    const secret = 'a-secret-that-is-at-least-32-bytes-long';
+    process.env.NEXT_PUBLIC_CHECK_SECRET = secret;
+    try {
+      const problems = checkSecuritySettings({ sessionSecrets: [secret] });
+      expect(problems).toHaveLength(1);
+      expect(problems[0].message).toContain('NEXT_PUBLIC_CHECK_SECRET');
+    } finally {
+      delete process.env.NEXT_PUBLIC_CHECK_SECRET;
+    }
+  });
+});