Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/security-startup-log.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chatcops/server": minor
---

The server logs its security settings once at startup (where the chat is allowed from, session tokens, Turnstile), plus an error or a warning for mistakes it can spot: an allowed origin without `http://` or `https://`, or with a path, query, fragment or trailing slash; an `http://` origin that isn't local; `"*"` next to other origins; a session secret that's also the value of a `NEXT_PUBLIC_`, `VITE_`, `PUBLIC_`, `NUXT_PUBLIC_`, `EXPO_PUBLIC_` or `REACT_APP_` variable; two secrets that are the same.
15 changes: 14 additions & 1 deletion packages/server/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import {
} from '@chatcops/core';
import type { ChatCopsServerConfig, ValidatedChatRequest } from './config.js';
import { chatRequestSchema } from './config.js';
import { createSessions } from './security/sessions.js';
import { allowedOriginsOf } from './security/origin-policy.js';
import { DEFAULT_SESSION_TTL_SEC, createSessions } from './security/sessions.js';
import { logSecurityAtStartup } from './security/startup-checks.js';
import { VISITOR_CONTEXT_INSTRUCTION, buildVisitorContext, isLocaleTag, withVisitorContext } from './visitor-context.js';
import { WebhookDispatcher } from './webhook.js';

Expand Down Expand Up @@ -97,6 +99,16 @@ export function createChatHandler(config: ChatCopsServerConfig) {
validateTools(toolDefs);
const tools = toolRegistry(config.tools);
const sessions = createSessions(config, async (id) => (await conversations.getMessages(id)).length === 0);
if (!loggedSecurity) {
loggedSecurity = true;
logSecurityAtStartup({
allowedOrigins: allowedOriginsOf(config),
sessionSecrets: config.sessionSecrets,
sessionTtlSec: sessions ? (config.sessionTtlSec ?? DEFAULT_SESSION_TTL_SEC) : undefined,
requireSession: sessions?.required,
turnstile: Boolean(config.turnstile),
});
}

const requests: Record<ChatFormat, number> = { v1: 0, legacy: 0, 'ai-sdk': 0 };

Expand Down Expand Up @@ -519,6 +531,7 @@ const DAY_MS = 24 * 60 * 60 * 1000;
/** Once per process (per isolate on Workers), not once per handler. */
let warnedMemoryStorage = false;
let loggedBudgets = false;
let loggedSecurity = false;
let loggedLegacyFormat = false;

const SCOPE_LABELS: Record<BudgetScope, string> = { conversation: 'per conversation', ip: 'per IP per day', global: 'per day in total' };
Expand Down
46 changes: 38 additions & 8 deletions packages/server/src/security/startup-checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ export function checkSecuritySettings(
}
}

for (const [index, origin] of origins.entries()) {
for (const origin of origins) {
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://.`,
message: `Allowed origin "${origin}" must start with http:// or https://.`,
});
continue;
}
Expand All @@ -109,33 +109,63 @@ export function checkSecuritySettings(
} catch {
problems.push({
level: 'error',
message: `Allowed origin ${index + 1} is not a valid URL.`,
message: `Allowed origin "${origin}" is not a valid URL.`,
});
continue;
}
if (parsed.pathname !== '/' || parsed.search !== '' || origin.endsWith('/')) {
if (parsed.pathname !== '/' || parsed.search !== '' || parsed.hash !== '' || /[/?#]$/.test(origin)) {
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.',
`Allowed origin "${origin}" must not have a path, a query, a fragment or a trailing slash, ` +
`because browsers send the origin without one. Use "${parsed.origin}".`,
});
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.`,
message: `Allowed origin "${origin}" 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.`,
message: 'Allowed origins include "*", which allows every site, so the other entries do nothing.',
});
}

return problems;
}

/** What a server's security settings come to, for the startup log. */
export interface SecuritySummary {
allowedOrigins: string[];
/** Seconds, when session tokens are on. */
sessionTtlSec?: number;
requireSession?: boolean;
turnstile: boolean;
}

/** The one line the server logs at startup about its security settings. */
export function describeSecurity(summary: SecuritySummary): string {
const origins = summary.allowedOrigins.includes('*')
? 'any site (allowedOrigins is "*"; list your sites in production)'
: summary.allowedOrigins.join(', ');
const sessions =
summary.sessionTtlSec === undefined
? 'session tokens off'
: `session tokens on (${summary.sessionTtlSec}s${summary.requireSession === false ? ', not required' : ''})`;
return `[chatcops] Security: chat allowed from ${origins}; ${sessions}; Turnstile ${summary.turnstile ? 'on' : 'off'}.`;
}

/** Logs the problems `checkSecuritySettings` finds, errors first, then the summary line. */
export function logSecurityAtStartup(settings: SecuritySettings & Omit<SecuritySummary, 'allowedOrigins'>): void {
const allowedOrigins = settings.allowedOrigins ?? ['*'];
const problems = checkSecuritySettings({ ...settings, allowedOrigins });
for (const problem of problems.filter((p) => p.level === 'error')) console.error(`[chatcops] ERROR: ${problem.message}`);
for (const problem of problems.filter((p) => p.level === 'warning')) console.warn(`[chatcops] ${problem.message}`);
console.info(describeSecurity({ ...settings, allowedOrigins }));
}
67 changes: 57 additions & 10 deletions packages/server/tests/security/startup-checks.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, it, expect } from 'vitest';
import { checkSecuritySettings } from '../../src/security/startup-checks.js';
import { afterEach, describe, it, expect, vi } from 'vitest';
import { scriptedProvider } from '@chatcops/core/testing/provider';
import { checkSecuritySettings, describeSecurity } from '../../src/security/startup-checks.js';

const LONG_SECRET = 'a-secret-that-is-at-least-32-bytes-long';

Expand Down Expand Up @@ -85,29 +86,34 @@ describe('checkSecuritySettings', () => {
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://.' },
{ level: 'error', message: 'Allowed origin "acme.com" 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.' },
{ level: 'error', message: 'Allowed origin "https://" is not a valid URL.' },
]);
});

it('reports an allowed origin with a path, a query or a trailing slash', () => {
it('reports an allowed origin with a path, a query, a fragment or a trailing slash, and says what to use', () => {
const problems = checkSecuritySettings(
{ allowedOrigins: ['https://acme.com/app', 'https://acme.com/?a=1', 'https://acme.com/'] },
{ allowedOrigins: ['https://acme.com/app', 'https://acme.com/?a=1', 'https://acme.com/', 'https://acme.com#top', 'https://acme.com?'] },
{},
);
expect(problems).toHaveLength(3);
expect(problems).toHaveLength(5);
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');
expect(problem.message).toContain('must not have a path, a query, a fragment or a trailing slash');
expect(problem.message).toContain('Use "https://acme.com".');
}
});

it('accepts subdomain wildcards and ports', () => {
expect(checkSecuritySettings({ allowedOrigins: ['https://*.acme.com', 'https://acme.com:8443'] }, {})).toEqual([]);
});

it('allows a plain http:// origin on localhost', () => {
const problems = checkSecuritySettings(
{ allowedOrigins: ['http://localhost:5173', 'http://127.0.0.1:8080'] },
Expand All @@ -121,15 +127,15 @@ describe('checkSecuritySettings', () => {
expect(problems).toEqual([
{
level: 'warning',
message: 'Allowed origin 1 uses http://. Use https://, unless it is a local development server.',
message: 'Allowed origin "http://acme.com" 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.' },
{ level: 'warning', message: 'Allowed origins include "*", which allows every site, so the other entries do nothing.' },
]);
});

Expand All @@ -149,3 +155,44 @@ describe('checkSecuritySettings', () => {
}
});
});

describe('the startup log', () => {
afterEach(() => {
vi.restoreAllMocks();
});

it('sums up the settings in one line', () => {
expect(describeSecurity({ allowedOrigins: ['*'], turnstile: false })).toBe(
'[chatcops] Security: chat allowed from any site (allowedOrigins is "*"; list your sites in production); session tokens off; Turnstile off.'
);
expect(describeSecurity({ allowedOrigins: ['https://acme.com', 'https://*.acme.com'], sessionTtlSec: 900, requireSession: false, turnstile: true })).toBe(
'[chatcops] Security: chat allowed from https://acme.com, https://*.acme.com; session tokens on (900s, not required); Turnstile on.'
);
});

it('logs errors, then warnings, then the summary, once per process', async () => {
vi.resetModules();
const error = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
const info = vi.spyOn(console, 'info').mockImplementation(() => undefined);
const { createChatHandler } = await import('../../src/handler.js');
const config = {
provider: scriptedProvider([]),
systemPrompt: 'x',
budgets: false as const,
allowedOrigins: ['acme.com', 'http://staging.acme.com'],
sessionSecrets: ['a-session-secret-that-is-long-enough-1'],
};

createChatHandler(config).cleanup();
createChatHandler(config).cleanup();

expect(error.mock.calls).toEqual([['[chatcops] ERROR: Allowed origin "acme.com" must start with http:// or https://.']]);
expect(warn.mock.calls).toEqual([
['[chatcops] Allowed origin "http://staging.acme.com" uses http://. Use https://, unless it is a local development server.'],
]);
expect(info.mock.calls.filter(([line]) => String(line).startsWith('[chatcops] Security:'))).toEqual([
['[chatcops] Security: chat allowed from acme.com, http://staging.acme.com; session tokens on (900s); Turnstile off.'],
]);
});
});
4 changes: 2 additions & 2 deletions packages/server/tests/storage-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ describe('default budgets', () => {
const { createChatHandler: fresh } = await import('../src/handler.js');
fresh({ provider: scriptedProvider([]), systemPrompt: 'x' }).cleanup();
fresh({ provider: scriptedProvider([]), systemPrompt: 'x' }).cleanup();
expect(info.mock.calls).toEqual([
expect(info.mock.calls.filter(([line]) => String(line).includes('Budgets'))).toEqual([
[
'[chatcops] Budgets: 200 replies per conversation (default), 2M tokens per IP per day (default). No daily total is set; `budgets.globalPerDay` caps your whole bill. Set `budgets` to change them, or `budgets: false` to turn them off.',
],
Expand All @@ -186,7 +186,7 @@ describe('default budgets', () => {
const info = vi.spyOn(console, 'info').mockImplementation(() => {});
const { createChatHandler: fresh } = await import('../src/handler.js');
fresh({ provider: scriptedProvider([]), systemPrompt: 'x', budgets: false }).cleanup();
expect(info).not.toHaveBeenCalled();
expect(info.mock.calls.filter(([line]) => String(line).includes('Budgets'))).toEqual([]);
});

it('describes budgets you set', () => {
Expand Down
9 changes: 9 additions & 0 deletions website/src/content/docs/server/security.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ With `turnstile` on the server, give the widget the site key. Nothing else chang

## Before you go live

The server logs one line about these settings when it starts, and an error or warning for the mistakes it can spot: an origin without `https://` or with a path, an `http://` origin that isn't local, a session secret that's also in a `NEXT_PUBLIC_` (or `VITE_`, `PUBLIC_` and so on) variable. For example:

```
[chatcops] ERROR: Allowed origin "acme.com" must start with http:// or https://.
[chatcops] Security: chat allowed from acme.com; session tokens on (900s); Turnstile off.
```

Then go through the list:

- `allowedOrigins` lists your sites, and nothing else. `['*']` is the default.
- `storage` is a store package, so limits and budgets hold across instances.
- `budgets.globalPerDay` is set to a number you're fine paying on a bad day.
Expand Down
Loading