From cf0af53dd410ab7329af4189066ca9301ffc5fc4 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 14 Apr 2026 02:42:32 +0200 Subject: [PATCH] Security hardening: low-risk easy wins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - JWT_SECRET fails fast at startup in production - CORS fails closed if APP_URL + CORS_ORIGINS are both missing - Explicit HSTS (1y, includeSubDomains, preload) - Rate limit sensitive auth endpoints (change-password, 2FA) - /api/health now returns {ok:true}; details gated behind admin auth - Login enumeration removed — generic 401 + dummy bcrypt on miss - ReDoS guard: 20KB input cap on /suggest-codes - showToast uses textContent, no innerHTML - clearSession() clears service worker caches on logout - OIDC state is now HMAC-signed and stateless (survives restart) - SSRF guard on admin-set OIDC issuer (blocks private IPs, requires HTTPS) Adds docs/mobile-build.md covering APK build, release, git push, keystore, and troubleshooting for both PedScribe and PedsHub apps. --- docs/mobile-build.md | 115 +++++++++++++++++++++++++++++++++++++++++ public/js/app.js | 5 +- public/js/auth.js | 9 ++++ server.js | 27 +++++++++- src/middleware/auth.js | 7 +++ src/routes/auth.js | 20 ++++--- src/routes/billing.js | 4 ++ src/routes/oidc.js | 87 ++++++++++++++++++++++--------- 8 files changed, 239 insertions(+), 35 deletions(-) create mode 100644 docs/mobile-build.md diff --git a/docs/mobile-build.md b/docs/mobile-build.md new file mode 100644 index 0000000..f122fd2 --- /dev/null +++ b/docs/mobile-build.md @@ -0,0 +1,115 @@ +# Mobile App Build & Release + +Capacitor wrappers for **PedScribe** (this repo) and **PedsHub Quiz** +(`/home/danvics/docker/quiz`). Both ship as Android APKs and iOS builds. + +## One-time setup + +- **Keystore** (reused for both apps): + ```bash + keytool -genkeypair -v -keystore ~/pedscribe-release.jks \ + -keyalg RSA -keysize 2048 -validity 10000 -alias pedscribe + ``` + Store the password somewhere safe — losing it means rotating signing keys. + +- **Android Studio path** (required when you want to open the IDE): + ```bash + export CAPACITOR_ANDROID_STUDIO_PATH="/snap/android-studio/209/bin/studio.sh" + ``` + Put it in your `~/.bashrc` if you want it permanent. + +## Release build — PedScribe + +```bash +cd /home/danvics/docker/ped-ai/mobile +npm install # picks up any new plugins +npx cap sync android # copies web assets + plugin glue +cd android +./gradlew assembleRelease \ + -Pandroid.injected.signing.store.file=$HOME/pedscribe-release.jks \ + -Pandroid.injected.signing.store.password=YOUR_KEYSTORE_PASSWORD \ + -Pandroid.injected.signing.key.alias=pedscribe \ + -Pandroid.injected.signing.key.password=YOUR_KEY_PASSWORD +# APK lands at: android/app/build/outputs/apk/release/app-release.apk +``` + +## Release build — PedsHub Quiz + +```bash +cd /home/danvics/docker/quiz/mobile +npm install +npx cap sync android +cd android +./gradlew assembleRelease \ + -Pandroid.injected.signing.store.file=$HOME/pedscribe-release.jks \ + -Pandroid.injected.signing.store.password=YOUR_KEYSTORE_PASSWORD \ + -Pandroid.injected.signing.key.alias=pedscribe \ + -Pandroid.injected.signing.key.password=YOUR_KEY_PASSWORD +``` + +## Publish APK on GitHub Releases + +The login page links to `github.com///releases/latest` (see +`public/index.html` around line 114). Publishing a tagged release updates the +download link automatically — no site redeploy needed. + +```bash +cd /home/danvics/docker/ped-ai +gh release create v6.0.1 \ + mobile/android/app/build/outputs/apk/release/app-release.apk \ + --title "PedScribe 6.0.1" \ + --notes "Hardware-backed secure storage for auth token on mobile." +``` + +For the quiz app: +```bash +cd /home/danvics/docker/quiz +gh release create v1.0.0 \ + mobile/android/app/build/outputs/apk/release/app-release.apk \ + --title "PedsHub 1.0.0" \ + --notes "Initial Android release." +``` + +## Push source changes to git + +Standard flow — the mobile project lives alongside the web app: + +```bash +cd /home/danvics/docker/ped-ai +git add mobile/ public/ src/ +git commit -m "Your message" +git push +``` + +Same for quiz at `/home/danvics/docker/quiz`. + +## iOS + +iOS requires macOS + Xcode. On Linux the sync still runs but you cannot build +the `.ipa`: + +```bash +cd /home/danvics/docker/ped-ai/mobile +npx cap sync ios +# Then on a Mac: open ios/App/App.xcworkspace and Archive. +``` + +## Reinstall on device after rebuild + +```bash +adb install -r android/app/build/outputs/apk/release/app-release.apk +``` + +`-r` preserves app data (saved server URL, cached sessions). + +## Troubleshooting + +- **`npx cap` can't find the project** — you must `cd mobile/` first, not run + from the repo root. +- **`Keystore was tampered with`** — wrong password. Do not generate a new + keystore unless you are ready to rotate the signing identity on Play Store. +- **Microphone "denied" in the app** — open system settings, long-press the + app icon → App info → Permissions → Microphone → Allow. Web-side prompt + does not always surface because the native layer intercepts it. +- **Foreground recording stops on newer Android** — the service must declare + `foregroundServiceType="microphone"` in `AndroidManifest.xml`. diff --git a/public/js/app.js b/public/js/app.js index 5c22a45..87bf6c4 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -516,7 +516,10 @@ function showToast(message, type) { var toast = document.createElement('div'); toast.className = 'toast toast-' + (type || 'success'); var icon = type === 'error' ? 'exclamation-circle' : type === 'info' ? 'info-circle' : 'check-circle'; - toast.innerHTML = ' ' + message; + var iconEl = document.createElement('i'); + iconEl.className = 'fas fa-' + icon; + toast.appendChild(iconEl); + toast.appendChild(document.createTextNode(' ' + String(message == null ? '' : message))); container.appendChild(toast); setTimeout(function() { toast.remove(); }, 3500); } diff --git a/public/js/auth.js b/public/js/auth.js index bc8a03f..9e2b73d 100644 --- a/public/js/auth.js +++ b/public/js/auth.js @@ -216,6 +216,15 @@ document.addEventListener('DOMContentLoaded', function() { localStorage.removeItem(SESSION_KEY); } window.AUTH_TOKEN = null; + // Clear service worker caches so a logged-out user on a shared device + // cannot read cached pages from offline mode. + try { + if (window.caches && caches.keys) { + caches.keys().then(function(names) { + names.forEach(function(n) { caches.delete(n); }); + }).catch(function(){}); + } + } catch(e) {} } window.getAuthHeaders = function() { diff --git a/server.js b/server.js index a70e216..69d15fa 100644 --- a/server.js +++ b/server.js @@ -19,6 +19,7 @@ app.set('trust proxy', 1); // ============================================================ app.use(helmet({ crossOriginEmbedderPolicy: false, + hsts: { maxAge: 31536000, includeSubDomains: true, preload: true }, contentSecurityPolicy: { directives: { defaultSrc: ["'self'"], @@ -53,12 +54,17 @@ app.use(helmet({ var allowedOrigins = []; if (process.env.APP_URL) allowedOrigins.push(process.env.APP_URL.replace(/\/$/, '')); if (process.env.CORS_ORIGINS) process.env.CORS_ORIGINS.split(',').forEach(function(o) { allowedOrigins.push(o.trim().replace(/\/$/, '')); }); +var IS_PROD = process.env.NODE_ENV === 'production' || !!process.env.APP_URL; +if (IS_PROD && allowedOrigins.length === 0) { + console.error('[FATAL] Production mode but no APP_URL or CORS_ORIGINS set. Refusing to run with open CORS.'); + process.exit(1); +} app.use(cors({ origin: function(origin, callback) { // Allow requests with no origin (mobile apps, curl, server-to-server) if (!origin) return callback(null, true); - // In development (no origins configured) allow all - if (allowedOrigins.length === 0) return callback(null, true); + // Development only: if no origins configured, allow all + if (!IS_PROD && allowedOrigins.length === 0) return callback(null, true); if (allowedOrigins.indexOf(origin) !== -1) return callback(null, true); callback(new Error('CORS: origin not allowed')); }, @@ -105,6 +111,17 @@ app.use('/api/auth/resend-verification', rateLimit({ message: { error: 'Too many verification requests. Try again in 15 minutes.' }, standardHeaders: true, legacyHeaders: false })); +// Post-login sensitive endpoints — protect against brute force with a stolen cookie. +// Keyed per-IP; 20 attempts / 15 min covers legitimate use, blocks TOTP brute force (10^6 space). +var sensitiveAuthLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, max: 20, + message: { error: 'Too many attempts. Try again in 15 minutes.' }, + standardHeaders: true, legacyHeaders: false +}); +app.use('/api/auth/change-password', sensitiveAuthLimiter); +app.use('/api/auth/setup-2fa', sensitiveAuthLimiter); +app.use('/api/auth/verify-2fa', sensitiveAuthLimiter); +app.use('/api/auth/disable-2fa', sensitiveAuthLimiter); // Serve .well-known/assetlinks.json for TWA verification (must be before static) app.get('/.well-known/assetlinks.json', (req, res) => { @@ -158,7 +175,13 @@ app.get('/api/models', async (req, res) => { }); const { activeProvider } = require('./src/utils/ai'); +// Public health: minimal — just confirm the server is up. app.get('/api/health', (req, res) => { + res.json({ ok: true }); +}); +// Detailed health: admin-only — exposes which providers are configured. +const { authMiddleware: _hcAuth, adminMiddleware: _hcAdmin } = require('./src/middleware/auth'); +app.get('/api/health/detailed', _hcAuth, _hcAdmin, (req, res) => { res.json({ status: 'running', version: '6.0.0', provider: activeProvider, timestamp: new Date().toISOString(), diff --git a/src/middleware/auth.js b/src/middleware/auth.js index 4bbfb05..da02ce9 100644 --- a/src/middleware/auth.js +++ b/src/middleware/auth.js @@ -2,6 +2,13 @@ const jwt = require('jsonwebtoken'); const db = require('../db/database'); const { hashToken } = require('../utils/sessions'); +if (!process.env.JWT_SECRET) { + if (process.env.NODE_ENV === 'production' || process.env.APP_URL) { + console.error('[FATAL] JWT_SECRET is not set. Refusing to start in production.'); + process.exit(1); + } + console.warn('[WARN] JWT_SECRET not set — using development fallback. DO NOT use in production.'); +} const JWT_SECRET = process.env.JWT_SECRET || 'dev-secret-change-in-production'; async function authMiddleware(req, res, next) { diff --git a/src/routes/auth.js b/src/routes/auth.js index 78e2aa0..40d643b 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -300,17 +300,23 @@ router.post('/login', async (req, res) => { } var user = await db.get('SELECT * FROM users WHERE email = ?', [email.toLowerCase()]); - if (!user) return res.status(401).json({ error: 'Invalid credentials' }); - - if (user.disabled) { - await db.run('INSERT INTO audit_log (user_id, action, ip_address, details) VALUES (?, ?, ?, ?)', - [user.id, 'login_blocked', req.ip, 'Account disabled']); - return res.status(403).json({ error: 'Account disabled. Contact an administrator.' }); + // Enumeration-resistant: always run bcrypt to keep timing constant, and return + // the same generic message for unknown-user, wrong-password, disabled, unverified. + var DUMMY_HASH = '$2b$12$CwTycUXWue0Thq9StjUM0uJ8.aDLA18dY6nB5xuWz5M6l8lR6rYS.'; + if (!user) { + await bcrypt.compare(password, DUMMY_HASH).catch(function(){}); + return res.status(401).json({ error: 'Invalid credentials' }); } var valid = await bcrypt.compare(password, user.password); if (!valid) { - await db.run('INSERT INTO audit_log (user_id, action, ip_address) VALUES (?, ?, ?)', [user.id, 'login_failed', req.ip]); + await db.run('INSERT INTO audit_log (user_id, action, ip_address) VALUES (?, ?, ?)', [user.id, 'login_failed', req.ip]).catch(function(){}); + return res.status(401).json({ error: 'Invalid credentials' }); + } + + if (user.disabled) { + await db.run('INSERT INTO audit_log (user_id, action, ip_address, details) VALUES (?, ?, ?, ?)', + [user.id, 'login_blocked', req.ip, 'Account disabled']).catch(function(){}); return res.status(401).json({ error: 'Invalid credentials' }); } diff --git a/src/routes/billing.js b/src/routes/billing.js index 2f0471d..fe928dc 100644 --- a/src/routes/billing.js +++ b/src/routes/billing.js @@ -285,6 +285,10 @@ router.post('/suggest-codes', authMiddleware, async function(req, res) { try { var { noteText, noteType, patientAge, visitType } = req.body; if (!noteText) return res.status(400).json({ error: 'No note text provided' }); + // ReDoS guard — regex over note text can blow up on pathological inputs. + if (typeof noteText !== 'string' || noteText.length > 20000) { + return res.status(400).json({ error: 'Note too long for billing analysis (max 20,000 chars)' }); + } // 1. Extract diagnoses from note text var diagnoses = extractDiagnoses(noteText); diff --git a/src/routes/oidc.js b/src/routes/oidc.js index aadb015..547c796 100644 --- a/src/routes/oidc.js +++ b/src/routes/oidc.js @@ -7,20 +7,62 @@ var express = require('express'); var router = express.Router(); var crypto = require('crypto'); var jwt = require('jsonwebtoken'); +var dns = require('dns').promises; +var net = require('net'); var db = require('../db/database'); var { JWT_SECRET, authMiddleware, adminMiddleware } = require('../middleware/auth'); var { hashToken, parseUserAgent, generateSessionId } = require('../utils/sessions'); -// In-memory OIDC state store (short-lived, no DB needed) -var pendingStates = {}; +// SSRF guard: reject issuer URLs that resolve to private / loopback / link-local IPs. +// Prevents an admin (or compromised admin account) from pointing OIDC at AWS metadata etc. +function isPrivateIp(ip) { + if (!ip) return true; + if (net.isIPv4(ip)) { + var p = ip.split('.').map(Number); + if (p[0] === 10) return true; + if (p[0] === 127) return true; + if (p[0] === 0) return true; + if (p[0] === 169 && p[1] === 254) return true; + if (p[0] === 172 && p[1] >= 16 && p[1] <= 31) return true; + if (p[0] === 192 && p[1] === 168) return true; + if (p[0] >= 224) return true; + return false; + } + if (net.isIPv6(ip)) { + var low = ip.toLowerCase(); + if (low === '::1' || low === '::' ) return true; + if (low.startsWith('fe80:') || low.startsWith('fc') || low.startsWith('fd')) return true; + return false; + } + return true; +} +async function assertSafeIssuer(urlStr) { + var u = new URL(urlStr); + if (u.protocol !== 'https:') throw new Error('OIDC issuer must use https://'); + var addrs = await dns.lookup(u.hostname, { all: true }); + for (var i = 0; i < addrs.length; i++) { + if (isPrivateIp(addrs[i].address)) throw new Error('OIDC issuer resolves to a private IP (blocked)'); + } +} -// Clean expired states every 10 minutes -setInterval(function() { - var now = Date.now(); - Object.keys(pendingStates).forEach(function(k) { - if (pendingStates[k].expires < now) delete pendingStates[k]; - }); -}, 10 * 60 * 1000); +// Signed, stateless OIDC state — survives restart / scales to multiple processes. +function signState(payload) { + var body = Buffer.from(JSON.stringify(payload)).toString('base64url'); + var sig = crypto.createHmac('sha256', JWT_SECRET).update(body).digest('base64url'); + return body + '.' + sig; +} +function verifyState(token) { + if (!token || typeof token !== 'string') return null; + var parts = token.split('.'); + if (parts.length !== 2) return null; + var expected = crypto.createHmac('sha256', JWT_SECRET).update(parts[0]).digest('base64url'); + if (!crypto.timingSafeEqual(Buffer.from(parts[1]), Buffer.from(expected))) return null; + try { + var payload = JSON.parse(Buffer.from(parts[0], 'base64url').toString('utf8')); + if (!payload.expires || payload.expires < Date.now()) return null; + return payload; + } catch (e) { return null; } +} function setAuthCookie(res, token) { var isProduction = process.env.NODE_ENV === 'production' || process.env.APP_URL; @@ -69,19 +111,19 @@ router.get('/oidc', async function(req, res) { var appUrl = (process.env.APP_URL || 'http://localhost:3000').replace(/\/$/, ''); var redirectUri = appUrl + '/api/auth/oidc/callback'; + await assertSafeIssuer(issuer); var config = await oidc.discovery(new URL(issuer), clientId); - var state = crypto.randomBytes(24).toString('hex'); var nonce = crypto.randomBytes(24).toString('hex'); var codeVerifier = oidc.randomPKCECodeVerifier(); var codeChallenge = await oidc.calculatePKCECodeChallenge(codeVerifier); - // Store state for callback verification (5 min TTL) - pendingStates[state] = { - nonce: nonce, - codeVerifier: codeVerifier, + // Stateless signed state — survives restart, works across multiple processes. + var state = signState({ + n: nonce, + v: codeVerifier, expires: Date.now() + 5 * 60 * 1000 - }; + }); var authUrl = oidc.buildAuthorizationUrl(config, { redirect_uri: redirectUri, @@ -105,17 +147,11 @@ router.get('/oidc/callback', async function(req, res) { try { var state = req.query.state; - if (!state || !pendingStates[state]) { + var pending = verifyState(state); + if (!pending) { return res.redirect(appUrl + '?error=invalid_state'); } - var pending = pendingStates[state]; - delete pendingStates[state]; - - if (pending.expires < Date.now()) { - return res.redirect(appUrl + '?error=expired'); - } - var issuer = await db.getSetting('oidc.issuer'); var clientId = await db.getSetting('oidc.client_id'); var clientSecret = await db.getSetting('oidc.client_secret'); @@ -123,11 +159,12 @@ router.get('/oidc/callback', async function(req, res) { var oidc = require('openid-client'); var redirectUri = appUrl + '/api/auth/oidc/callback'; + await assertSafeIssuer(issuer); var config = await oidc.discovery(new URL(issuer), clientId, clientSecret || undefined); var tokens = await oidc.authorizationCodeGrant(config, new URL(req.protocol + '://' + req.get('host') + req.originalUrl), { - pkceCodeVerifier: pending.codeVerifier, - expectedNonce: pending.nonce, + pkceCodeVerifier: pending.v, + expectedNonce: pending.n, expectedState: state });