From 5d4f0114cd89c7879a19327c9a283c6dccdc875c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 25 Feb 2026 19:44:34 +0000 Subject: [PATCH] Critical Securities (5) fixes --- .env.example | 2 +- SECURITY_FIXES.md | 81 +++++++ bun.lock | 5 + package.json | 1 + src/server.tsx | 32 ++- src/servers/_middlewares/index.ts | 1 + src/servers/_middlewares/origin-validation.ts | 33 +-- .../_middlewares/super-admin-middleware.ts | 8 + src/servers/api/dev-server/router.ts | 200 +++++++++++------- .../api/server-settings/server-settings.ts | 2 +- src/servers/hono.ts | 15 +- src/servers/jwt.ts | 5 + .../apps/Chat/components/MessageBubble.tsx | 14 +- .../src/apps/Preview/PreviewProvider.tsx | 6 +- 14 files changed, 302 insertions(+), 103 deletions(-) create mode 100644 SECURITY_FIXES.md create mode 100644 src/servers/_middlewares/super-admin-middleware.ts diff --git a/.env.example b/.env.example index 3a78b786..c3aa4894 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,5 @@ PORT=9000 -JWT_SECRET="change-me" +JWT_SECRET="" POSTGRES_URL="postgres://postgres:password@localhost:5432/officer" MAIL_TRANSPORT="smtp://localhost:1025" PUBLIC_URL=http://localhost:9000 diff --git a/SECURITY_FIXES.md b/SECURITY_FIXES.md new file mode 100644 index 00000000..7e4425c1 --- /dev/null +++ b/SECURITY_FIXES.md @@ -0,0 +1,81 @@ +# Security Fixes Log + +Tracks remediation progress against findings in `SECURITY_AUDIT.md`. + +--- + +## C1. Weak JWT Secret — FIXED + +**Ref:** SECURITY_AUDIT.md > C1 + +**Changes:** +- `src/servers/jwt.ts` — Added startup guard that throws if `JWT_SECRET` is absent or shorter than 32 characters +- `.env.example` — Updated placeholder to `` + +**Note:** The actual `.env` secret must be rotated manually per deployment. + +--- + +## C2. Global CORS Wildcard on All API Routes — FIXED + +**Ref:** SECURITY_AUDIT.md > C2 + +**Changes:** +- `src/servers/hono.ts` — Replaced `origin: '*'` with dynamic origin validation using the `ALLOWED_ORIGINS` allowlist from `origin-validation.ts` + +--- + +## C3. Server-Settings Routes Entirely Unauthenticated — FIXED + +**Ref:** SECURITY_AUDIT.md > C3 + +**Changes:** +- `src/servers/hono.ts` — Moved `serverSettingsRouter` inside `protectedRouter` +- `src/servers/api/server-settings/server-settings.ts` — Added Super Admin role checks on all write operations +- `src/servers/_middlewares/super-admin-middleware.ts` — New middleware for Super Admin enforcement +- Exempted `GET /api/server-settings/onboarding-complete` as public (mounted before auth) + +--- + +## C4. Unauthenticated Dev-Server WebSocket Proxy — FIXED + +**Ref:** SECURITY_AUDIT.md > C4 + +**Changes:** +- `src/servers/api/dev-server/router.ts`: + - Replaced slug-based proxy URLs with server-generated UUID v4 (`proxyId`). Proxy lookup is now by UUID, eliminating cross-user slug collisions. + - Added `proxyIdIndex` map for O(1) lookup and `pendingStarts` guard against duplicate spawns. + - JWT validation required on HTML requests (`Accept: text/html`) — the iframe entry point. Sub-resources pass on proxyId alone (browsers can't add custom headers to native loads). + - `?token=` query param stripped before forwarding to dev server. + - `proxyOverrideScript` extended to inject `Authorization: Bearer` headers on fetch/XHR and append `?token=` on WebSocket connections, reading fresh tokens from localStorage. +- `src/server.tsx`: + - WebSocket upgrades require `?token=` query param. Token stored in `WSData.wsToken` for deferred async validation in the `open` handler (Bun requires synchronous `server.upgrade()`). + - `devServerWebsocket.open` validates JWT + blacklist check before opening upstream connection. Closes with code `4001` if invalid. +- `src/workspaces/officerdev/src/apps/Preview/PreviewProvider.tsx`: + - `startServer` and status check `useEffect` append `?token=` to iframe URL using `client.token`. + +**Security model:** Two-layer capability — proxyId (unguessable UUID) as primary token, JWT as defense-in-depth on entry points (HTML, WebSocket). + +--- + +## C5. AI Chat Messages Rendered with `rehype-raw` — LLM-Driven XSS — FIXED + +**Ref:** SECURITY_AUDIT.md > C5 + +**Changes:** +- Installed `rehype-sanitize` (v6) +- `src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx`: + - Added `rehypeSanitize` with a schema that strips ``; // HTML: rewrite src/href/action attributes with absolute paths + inject fetch/XHR override diff --git a/src/servers/api/server-settings/server-settings.ts b/src/servers/api/server-settings/server-settings.ts index b98cf242..c732f111 100644 --- a/src/servers/api/server-settings/server-settings.ts +++ b/src/servers/api/server-settings/server-settings.ts @@ -36,7 +36,7 @@ serverSettingsRouter.route('/stt', sttRouter); serverSettingsRouter.route('/ocr', ocrRouter); serverSettingsRouter.route('/searxng', searxngRouter); -const readSettings = async () => { +export const readSettings = async () => { try { return await Bun.file(settingsPath).json(); } catch { return {}; } }; diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 6aece0c5..fa0be19c 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -25,7 +25,7 @@ import { integrationsRouter, googleCallbackHandler } from './api/integrations/in import { queueRouter } from './api/queue/queue'; import { emailRouter } from './api/email/email'; import { CustomError } from './custom-errors'; -import { userMiddleware, bodyParser } from './_middlewares'; +import { userMiddleware, bodyParser, isOriginAllowed, superAdminMiddleware } from './_middlewares'; export { Hono }; export { createRouter }; @@ -35,7 +35,10 @@ export const honoServer = new Hono<{ Variables: HonoVariables }>(); honoServer.use( cors({ - origin: '*', + origin: (origin, c) => { + const host = c.req.header('host'); + return isOriginAllowed(origin, host) ? origin : ''; + }, allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'], allowHeaders: ['Content-Type', 'Authorization'], }), @@ -43,16 +46,22 @@ honoServer.use( honoServer.get('/api', (ctx) => ctx.json({ officerAPI: 'ok' })); honoServer.route('/api/auth', authRouter); -honoServer.route('/api/server-settings', serverSettingsRouter); honoServer.route('/api/landing-page-data', landingPageDataRouter); honoServer.route('/api/waitlist', waitlistRouter); honoServer.route('/api/dev-server-proxy', devServerProxyRouter); honoServer.get('/api/integrations/google/callback', googleCallbackHandler); +honoServer.get('/api/server-settings/onboarding-complete', async (ctx) => { + const { readSettings } = await import('./api/server-settings/server-settings'); + const settings = await readSettings(); + return ctx.json({ onboardingComplete: !!settings.onboardingComplete }); +}); const protectedRouter = createRouter(); protectedRouter.use(bodyParser()); protectedRouter.use(userMiddleware); +serverSettingsRouter.use(superAdminMiddleware); +protectedRouter.route('/server-settings', serverSettingsRouter); protectedRouter.route('/users', usersRouter); protectedRouter.route('/plans', plansRouter); protectedRouter.route('/skills', skillsRouter); diff --git a/src/servers/jwt.ts b/src/servers/jwt.ts index 10ba4c85..231ac56e 100644 --- a/src/servers/jwt.ts +++ b/src/servers/jwt.ts @@ -1,6 +1,11 @@ import { sign as jwtSign, verify as jwtVerify } from 'hono/jwt'; + const { JWT_SECRET } = process.env; +if (!JWT_SECRET || JWT_SECRET.length < 32) { + throw new Error('JWT_SECRET must be set and at least 32 characters long. Generate one with: openssl rand -base64 32'); +} + function parseExpiration(expiration: string): number { const match = expiration.match(/^(\d+)([smhd])$/); if (!match) { diff --git a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx index 9fef3e7b..ed446dc6 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx @@ -2,6 +2,7 @@ import { useState, useRef } from 'react'; import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; import rehypeRaw from 'rehype-raw'; +import rehypeSanitize, { defaultSchema } from 'rehype-sanitize'; import { Volume2, Loader2, Square } from 'lucide-react'; import type { ChatMessage } from '../types'; import { ToolActivity } from './ToolActivity'; @@ -9,6 +10,15 @@ import { QuestionActivity } from './QuestionActivity'; import { getRawUrl } from '../../FileViewer/file-types'; import { useFilesAPI } from '../../../hooks/useFilesAPI'; +const sanitizeSchema = { + ...defaultSchema, + tagNames: (defaultSchema.tagNames ?? []).filter((tag) => tag !== 'script' && tag !== 'iframe' && tag !== 'object' && tag !== 'embed' && tag !== 'form'), + attributes: { + ...defaultSchema.attributes, + '*': (defaultSchema.attributes?.['*'] ?? []).filter((attr) => typeof attr === 'string' && !attr.startsWith('on')), + }, +}; + // Matches absolute image file paths, e.g. /home/user/pic.png or /tmp/photo.jpg const IMAGE_PATH_RE = /(\/(?:home\/[^/\s]+\/)?[^\s`"'<>\n\r[\]()]+\.(?:png|jpg|jpeg|gif|webp|svg|bmp|ico))/gi; @@ -129,7 +139,7 @@ export const MessageBubble = ({ message, onAnswer }: MessageBubbleProps) => {
- + {injectImages(assistantText)}
@@ -178,7 +188,7 @@ export const StreamingBubble = ({ text }: StreamingBubbleProps) => {
- + {injectImages(text)} diff --git a/src/workspaces/officerdev/src/apps/Preview/PreviewProvider.tsx b/src/workspaces/officerdev/src/apps/Preview/PreviewProvider.tsx index 28cf56ba..3ec09982 100644 --- a/src/workspaces/officerdev/src/apps/Preview/PreviewProvider.tsx +++ b/src/workspaces/officerdev/src/apps/Preview/PreviewProvider.tsx @@ -35,7 +35,8 @@ export const PreviewProvider = ({ children }: PreviewProviderProps) => { setStopped(false); try { const res = await client.post('/dev-server/start', { slug: targetSlug }); - setUrl(res.url); + const token = client.token; + setUrl(token ? `${res.url}?token=${encodeURIComponent(token)}` : res.url); setPort(res.port); } catch (err: unknown) { const msg = err && typeof err === 'object' && 'message' in err ? String(err.message) : 'Failed to start dev server'; @@ -81,7 +82,8 @@ export const PreviewProvider = ({ children }: PreviewProviderProps) => { const status = await client.get(`/dev-server/status?slug=${encodeURIComponent(slug)}`); if (cancelled) return; if (status.running && status.url) { - setUrl(status.url); + const token = client.token; + setUrl(token ? `${status.url}?token=${encodeURIComponent(token)}` : status.url); setPort(status.port ?? null); } else { startServer(slug);