diff --git a/CHANGELOG.md b/CHANGELOG.md index 373ee19c..cdf91b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Login loop caused by remote node auth failure:** When a configured remote node had an expired or invalid API token, every proxied request to that node returned 401. Both `apiFetch` and `fetchForNode` treated any 401 as a user session failure and dispatched `sencho-unauthorized`, immediately logging the user out and sending them back to the login page — even after a successful login. Fixed by adding a `proxyRes` handler that stamps all remote-proxied responses with `x-sencho-proxy: 1`; the frontend now only fires `sencho-unauthorized` when this header is absent (i.e., it's a genuine local session failure). +- **Missing `authMiddleware` on notifications endpoints:** `GET /api/notifications`, `POST /api/notifications/read`, `DELETE /api/notifications/:id`, `DELETE /api/notifications`, and `POST /api/notifications/test` were all missing `authMiddleware`, violating the default-deny policy. Added to all five. +- **CSP `workerSrc` missing (Monaco editor workers):** The Content Security Policy had no explicit `worker-src` directive, which in practice relied on `default-src 'self'`. Monaco editor creates Web Workers via `blob:` URLs for language services; these were silently failing. Added `worker-src 'self' blob:`. +- **CSP `connectSrc` implicit (WebSocket):** Added explicit `connect-src 'self' ws: wss:` to clearly permit same-origin WebSocket connections in both HTTP and HTTPS contexts. + ### Fixed - **Blank page on HTTP deployments (CSP `upgrade-insecure-requests`):** Helmet's default CSP included `upgrade-insecure-requests`, which causes browsers to silently upgrade all HTTP sub-resource fetches (JS, CSS, images) to HTTPS. On a plain-HTTP self-hosted deployment this produces a completely blank page with `ERR_SSL_PROTOCOL_ERROR`. The directive is now explicitly omitted from the CSP. - **HSTS permanently breaking HTTP access:** Helmet's default `Strict-Transport-Security` header was being sent over HTTP, instructing browsers to refuse non-HTTPS connections for 1 year. HSTS is now disabled and must only be re-enabled by users who terminate HTTPS at a reverse proxy in front of Sencho. diff --git a/backend/src/index.ts b/backend/src/index.ts index 46a0c095..65a8a257 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -87,6 +87,12 @@ app.use(helmet({ scriptSrc: ["'self'"], scriptSrcAttr: ["'none'"], styleSrc: ["'self'", 'https:', "'unsafe-inline'"], + // connect-src: explicit 'self' covers same-origin fetch/XHR/WebSocket. + // ws: and wss: are included for WebSocket connections in any scheme context. + connectSrc: ["'self'", 'ws:', 'wss:'], + // worker-src: Monaco editor creates Web Workers via blob: URLs for language + // services (syntax highlighting, intellisense). Without blob: they silently fail. + workerSrc: ["'self'", 'blob:'], // 'upgrade-insecure-requests' is intentionally absent — see comment above. }, }, @@ -450,6 +456,15 @@ const remoteNodeProxy = createProxyMiddleware({ // http-proxy's req.pipe(proxyReq) forwards the body automatically. // No manual body rewriting needed here. }, + proxyRes: (proxyRes) => { + // Mark every response forwarded from a remote node with a sentinel header. + // The frontend (apiFetch / fetchForNode) checks this before firing the + // global 'sencho-unauthorized' event: a 401 from a remote means the stored + // api_token for that node is invalid — not that the user's own session + // expired. Without this distinction, any node with a bad token causes an + // immediate logout loop. + proxyRes.headers['x-sencho-proxy'] = '1'; + }, error: (err, _req, proxyRes) => { console.error('[Proxy] Remote node error:', (err as Error).message); // proxyRes can be either a ServerResponse (HTTP) or a raw Socket (WS/TCP errors). @@ -1559,7 +1574,7 @@ app.delete('/api/alerts/:id', async (req: Request, res: Response) => { } }); -app.get('/api/notifications', async (req: Request, res: Response) => { +app.get('/api/notifications', authMiddleware, async (req: Request, res: Response) => { try { const history = DatabaseService.getInstance().getNotificationHistory(); res.json(history); @@ -1568,7 +1583,7 @@ app.get('/api/notifications', async (req: Request, res: Response) => { } }); -app.post('/api/notifications/read', async (req: Request, res: Response) => { +app.post('/api/notifications/read', authMiddleware, async (req: Request, res: Response) => { try { DatabaseService.getInstance().markAllNotificationsRead(); res.json({ success: true }); @@ -1577,7 +1592,7 @@ app.post('/api/notifications/read', async (req: Request, res: Response) => { } }); -app.delete('/api/notifications/:id', async (req: Request, res: Response) => { +app.delete('/api/notifications/:id', authMiddleware, async (req: Request, res: Response) => { try { const id = parseInt(req.params.id as string, 10); DatabaseService.getInstance().deleteNotification(id); @@ -1587,7 +1602,7 @@ app.delete('/api/notifications/:id', async (req: Request, res: Response) => { } }); -app.delete('/api/notifications', async (req: Request, res: Response) => { +app.delete('/api/notifications', authMiddleware, async (req: Request, res: Response) => { try { DatabaseService.getInstance().deleteAllNotifications(); res.json({ success: true }); @@ -1596,7 +1611,7 @@ app.delete('/api/notifications', async (req: Request, res: Response) => { } }); -app.post('/api/notifications/test', async (req: Request, res: Response) => { +app.post('/api/notifications/test', authMiddleware, async (req: Request, res: Response) => { try { const { type, url } = req.body; await NotificationService.getInstance().testDispatch(type, url); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 3282f165..c28cea5e 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -26,8 +26,13 @@ export async function apiFetch( const response = await fetch(url, { ...defaultOptions, ...fetchOptions }); if (response.status === 401) { - // Signal auth failure to AuthContext without a hard page reload - window.dispatchEvent(new Event('sencho-unauthorized')); + // Only fire the global logout event for local auth failures. + // When the response carries x-sencho-proxy, the 401 came from a remote + // Sencho node (expired/invalid api_token) — not from the user's own session. + // Logging out in that case creates an unrecoverable loop. + if (!response.headers.get('x-sencho-proxy')) { + window.dispatchEvent(new Event('sencho-unauthorized')); + } throw new Error('Unauthorized'); } @@ -66,7 +71,10 @@ export async function fetchForNode( }); if (response.status === 401) { - window.dispatchEvent(new Event('sencho-unauthorized')); + // Same logic as apiFetch: only log out for local auth failures. + if (!response.headers.get('x-sencho-proxy')) { + window.dispatchEvent(new Event('sencho-unauthorized')); + } throw new Error('Unauthorized'); }