From b4a808adcfd3bcee9205e8889a53b53e708d8109 Mon Sep 17 00:00:00 2001 From: arkon Date: Sat, 11 Apr 2026 07:20:09 +0200 Subject: [PATCH] fix: security hardening and cleanup from community PR cherry-picks - Add HTML sanitizer for markdown rendering (XSS prevention) - Switch service worker to network-first caching (deploys take effect immediately) - Sanitize Content-Disposition filenames (header injection prevention) - Expose session.muxName getter, replace unsafe `as any` cast - Static import for execFile, update CLAUDE.md keyboard shortcuts Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 4 ++-- src/session.ts | 5 +++++ src/web/public/app.js | 31 ++++++++++++++++++++++++++++++- src/web/public/sw.js | 15 +++++++-------- src/web/routes/file-routes.ts | 4 +++- src/web/routes/session-routes.ts | 4 ++-- 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5cd778b3..cb95a02f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,7 +110,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | **Infra** | `src/hooks-config.ts`, `src/push-store.ts`, `src/tunnel-manager.ts`, `src/image-watcher.ts`, `src/file-stream-manager.ts` | | | **Plan** | `src/plan-orchestrator.ts`, `src/prompts/*.ts`, `src/templates/claude-md.ts` | | | **Web** | `src/web/server.ts`, `src/web/sse-events.ts`, `src/web/routes/*.ts` (14 route modules + barrel), `src/web/route-helpers.ts`, `src/web/ports/*.ts`, `src/web/middleware/auth.ts`, `src/web/schemas.ts` | | -| **Frontend** | `src/web/public/app.js` (~2.6K lines, core) + 5 infra modules (`constants.js`, `mobile-handlers.js`, `voice-input.js`, `notification-manager.js`, `keyboard-accessory.js`) + 7 domain modules (`terminal-ui.js`, `respawn-ui.js`, `ralph-panel.js`, `orchestrator-panel.js`, `settings-ui.js`, `panels-ui.js`, `session-ui.js`) + 4 feature modules (`ralph-wizard.js`, `api-client.js`, `subagent-windows.js`, `input-cjk.js`) + `sw.js` | | +| **Frontend** | `src/web/public/app.js` (~2.8K lines, core) + 5 infra modules (`constants.js`, `mobile-handlers.js`, `voice-input.js`, `notification-manager.js`, `keyboard-accessory.js`) + 7 domain modules (`terminal-ui.js`, `respawn-ui.js`, `ralph-panel.js`, `orchestrator-panel.js`, `settings-ui.js`, `panels-ui.js`, `session-ui.js`) + 4 feature modules (`ralph-wizard.js`, `api-client.js`, `subagent-windows.js`, `input-cjk.js`) + `sw.js` | | | **Types** | `src/types/index.ts` → 14 domain files | See `@fileoverview` in index.ts | ★ = Large file (>50KB). All files have `@fileoverview` JSDoc — read that before diving in. @@ -150,7 +150,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **Respawn presets**: `solo-work` (3s/60min), `subagent-workflow` (45s/240min), `team-lead` (90s/480min), `ralph-todo` (8s/480min), `overnight-autonomous` (10s/480min). -**Keyboard shortcuts**: Escape (close), Ctrl+? (help), Ctrl+Enter (quick start), Ctrl+W (kill), Ctrl+Tab (next), Ctrl+K (kill all), Ctrl+L (clear), Ctrl+Shift+R (restore size), Ctrl/Cmd +/- (font). +**Keyboard shortcuts**: Escape (close), Ctrl+? (help), Ctrl+W (kill), Ctrl+Tab (next), Alt+1-9 (switch tab), Shift+Enter (newline), Ctrl+L (clear), Ctrl+Shift+R (restore size), Ctrl/Cmd +/- (font). ### Security diff --git a/src/session.ts b/src/session.ts index 918714b6..a47270a2 100644 --- a/src/session.ts +++ b/src/session.ts @@ -530,6 +530,11 @@ export class Session extends EventEmitter { return this._claudeSessionId; } + /** The tmux session name, if the session is running inside a mux */ + get muxName(): string | null { + return this._muxSession?.muxName ?? null; + } + get totalCost(): number { return this._totalCost; } diff --git a/src/web/public/app.js b/src/web/public/app.js index 5908d285..8dd00d49 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -898,11 +898,40 @@ class CodemanApp { // Response Viewer — native-scroll panel for reading full Claude responses // ═══════════════════════════════════════════════════════════════ + /** Strip dangerous elements and attributes from HTML (XSS prevention) */ + _sanitizeHtml(html) { + const tpl = document.createElement('template'); + tpl.innerHTML = html; + const frag = tpl.content; + // Remove dangerous elements + for (const el of frag.querySelectorAll('script, iframe, object, embed, form, base, meta, link, style')) { + el.remove(); + } + // Strip dangerous attributes from all elements + for (const el of frag.querySelectorAll('*')) { + for (const attr of [...el.attributes]) { + const name = attr.name.toLowerCase(); + if (name.startsWith('on')) { + el.removeAttribute(attr.name); + } else if (['href', 'src', 'action', 'xlink:href', 'formaction'].includes(name)) { + const val = attr.value.replace(/\s/g, '').toLowerCase(); + if (val.startsWith('javascript:') || val.startsWith('vbscript:') || val.startsWith('data:text/html')) { + el.removeAttribute(attr.name); + } + } + } + } + // Serialize back via a container + const div = document.createElement('div'); + div.appendChild(frag); + return div.innerHTML; + } + /** Render markdown to sanitized HTML, falling back to plain text if marked.js unavailable */ _renderMarkdown(text) { if (typeof marked !== 'undefined' && marked.parse) { try { - return marked.parse(text, { breaks: true, gfm: true }); + return this._sanitizeHtml(marked.parse(text, { breaks: true, gfm: true })); } catch { /* fall through */ } } // Fallback: escape HTML and preserve whitespace diff --git a/src/web/public/sw.js b/src/web/public/sw.js index 5c6e8039..ae887745 100644 --- a/src/web/public/sw.js +++ b/src/web/public/sw.js @@ -72,7 +72,9 @@ self.addEventListener('activate', (event) => { ); }); -// --- Fetch: network-first for API/navigation, cache-first for static --- +// --- Fetch: network-first with cache fallback --- +// Network-first ensures deploys take effect immediately when online. +// Cache is only used when the network is unavailable (offline/flaky). self.addEventListener('fetch', (event) => { const { request } = event; @@ -84,18 +86,15 @@ self.addEventListener('fetch', (event) => { if (request.url.includes('/api/')) return; event.respondWith( - caches.match(request).then((cached) => { - // Return cache immediately, refresh in background (stale-while-revalidate) - const fetchPromise = fetch(request).then((response) => { + fetch(request) + .then((response) => { if (response && response.ok) { const clone = response.clone(); caches.open(CACHE_NAME).then((cache) => cache.put(request, clone)); } return response; - }).catch(() => cached); - - return cached || fetchPromise; - }) + }) + .catch(() => caches.match(request)) ); }); diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index 5db1fbf6..c9b495c1 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -293,7 +293,9 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort): void const content = await fs.readFile(resolvedPath); if (download === 'true') { - const basename = filePath!.split('/').pop() || 'download'; + const rawBasename = filePath!.split('/').pop() || 'download'; + // Sanitize filename for Content-Disposition header (prevent header injection) + const basename = rawBasename.replace(/["\\\r\n]/g, '_'); reply.raw.writeHead(200, { 'Content-Type': mimeTypes[ext] || 'application/octet-stream', 'Content-Disposition': `attachment; filename="${basename}"`, diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index ddfd29d8..7f20c5d4 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -7,6 +7,7 @@ import { FastifyInstance } from 'fastify'; import { join, dirname } from 'node:path'; import { existsSync, statSync, mkdirSync, writeFileSync } from 'node:fs'; +import { execFile } from 'node:child_process'; import fs from 'node:fs/promises'; import { ApiErrorCode, @@ -545,13 +546,12 @@ export function registerSessionRoutes( } const session = findSessionOrFail(ctx, id); - const muxName = (session as any)._muxSession?.muxName; + const muxName = session.muxName; if (!muxName) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'No tmux session'); } try { - const { execFile } = await import('child_process'); await new Promise((resolve, reject) => { execFile('tmux', ['send-keys', '-H', '-t', muxName, ...hex], { timeout: 5000 }, (err) => { if (err) reject(err);