Skip to content

Commit cfddb90

Browse files
committed
Security, reliability, and hardening audit pass
Security fixes: - Remove hardcoded API credentials from VIPTrack CONFIG - Bind Ollama/Kiwix to Config.APP_HOST (127.0.0.1) instead of 0.0.0.0 - Replace naive replace('..','') folder sanitization in media uploads with proper posixpath.normpath-based _sanitize_folder() - Sanitize SSE event_type in broadcast_event to prevent injection Reliability fixes: - Add double-checked locking for WAL mode init in db.py (_wal_lock) - Use PASSIVE WAL checkpoint in backup_db to avoid blocking writers - Keep pool lock held during get_nowait in _pool_acquire (TOCTOU fix) - Fix zombie process leak in manager.py start_process error path (add proc.wait + stdout.close on DB failure) - Fix is_running race: keep _lock held during proc.poll() - Add double-checked locking for GPU detection (prevent concurrent subprocess spawns) - Make federation sync atomic: single commit for data + vector clocks + sync log instead of three separate commits - Wrap save_config with _config_lock to prevent concurrent write races - Fix db-vacuum endpoint for in-memory test databases (uri= parameter) Frontend hardening: - Add 30s AbortController timeout to safeFetch, apiFetch, and CSRF token fetch to prevent hanging on dead requests - Add 60s/30s timeouts to offline.js fullSync/incrementalSync fetches Performance: - Prune empty IP deques in mutating rate limiter to prevent unbounded memory growth from old client addresses Data integrity: - Add CHECK constraints for medical vitals (bp, pulse, resp, temp, spo2 0-100, pain 0-10, GCS 3-15) - Add CHECK constraints for patient age (0-200) and weight (0-700) Observability: - Log failed RSS feed worker exceptions with feed name instead of silently swallowing All 913 tests pass.
1 parent 4960d57 commit cfddb90

15 files changed

Lines changed: 133 additions & 65 deletions

File tree

config.py

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -160,31 +160,31 @@ def get_config_value(key: str, default=None):
160160
def save_config(data: dict):
161161
global _config_cache, _config_mtime
162162
path = get_config_path()
163-
os.makedirs(os.path.dirname(path), exist_ok=True)
164-
tmp_path = path + '.tmp'
165-
with open(tmp_path, 'w') as f:
166-
json.dump(data, f, indent=2)
167-
f.flush()
168-
os.fsync(f.fileno())
169-
import sys
170-
try:
171-
os.replace(tmp_path, path)
172-
except PermissionError:
173-
if sys.platform == 'win32':
174-
# Windows: retry with backoff — antivirus may briefly lock the file
175-
import time as _time
176-
for _attempt in range(3):
177-
_time.sleep(0.1 * (_attempt + 1))
178-
try:
179-
os.replace(tmp_path, path)
180-
break
181-
except PermissionError:
182-
if _attempt == 2:
183-
raise
184-
else:
185-
raise
186-
# Update cache immediately so subsequent reads are consistent
187163
with _config_lock:
164+
os.makedirs(os.path.dirname(path), exist_ok=True)
165+
tmp_path = path + '.tmp'
166+
with open(tmp_path, 'w') as f:
167+
json.dump(data, f, indent=2)
168+
f.flush()
169+
os.fsync(f.fileno())
170+
import sys
171+
try:
172+
os.replace(tmp_path, path)
173+
except PermissionError:
174+
if sys.platform == 'win32':
175+
# Windows: retry with backoff — antivirus may briefly lock the file
176+
import time as _time
177+
for _attempt in range(3):
178+
_time.sleep(0.1 * (_attempt + 1))
179+
try:
180+
os.replace(tmp_path, path)
181+
break
182+
except PermissionError:
183+
if _attempt == 2:
184+
raise
185+
else:
186+
raise
187+
# Update cache immediately so subsequent reads are consistent
188188
_config_cache = data
189189
try:
190190
_config_mtime = os.path.getmtime(path)

db.py

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def get_db_path():
5252

5353

5454
_wal_set = False
55+
_wal_lock = threading.Lock()
5556
_migration_lock = threading.Lock()
5657

5758

@@ -63,8 +64,10 @@ def get_db():
6364
conn.row_factory = sqlite3.Row
6465
# WAL mode is persistent on the database file — only set once per process
6566
if not _wal_set:
66-
conn.execute('PRAGMA journal_mode=WAL')
67-
_wal_set = True
67+
with _wal_lock:
68+
if not _wal_set:
69+
conn.execute('PRAGMA journal_mode=WAL')
70+
_wal_set = True
6871
conn.execute('PRAGMA foreign_keys=ON')
6972
# Register on flask.g so teardown_appcontext can auto-close leaked connections
7073
try:
@@ -97,10 +100,10 @@ def _pool_acquire():
97100
if _pool_db_path != current_path:
98101
_pool_clear()
99102
_pool_db_path = current_path
100-
try:
101-
conn = _pool.get_nowait()
102-
except queue.Empty:
103-
return get_db(), False
103+
try:
104+
conn = _pool.get_nowait()
105+
except queue.Empty:
106+
return get_db(), False
104107
try:
105108
conn.execute('SELECT 1').fetchone()
106109
# Rebind to current flask.g so teardown can see it
@@ -210,10 +213,11 @@ def backup_db():
210213
os.makedirs(backup_dir, exist_ok=True)
211214
from datetime import datetime
212215
backup_path = os.path.join(backup_dir, f'nomad_{datetime.now().strftime("%Y%m%d_%H%M%S")}.db')
213-
# Use SQLite backup API for WAL-safe copies
216+
# Use SQLite backup API for WAL-safe copies.
217+
# PASSIVE checkpoint avoids blocking concurrent writers.
214218
src = sqlite3.connect(db_path, timeout=30)
215219
try:
216-
src.execute('PRAGMA wal_checkpoint(RESTART)')
220+
src.execute('PRAGMA wal_checkpoint(PASSIVE)')
217221
dst = sqlite3.connect(backup_path)
218222
try:
219223
src.backup(dst)
@@ -878,8 +882,8 @@ def _create_medical_security_tables(conn):
878882
id INTEGER PRIMARY KEY AUTOINCREMENT,
879883
contact_id INTEGER,
880884
name TEXT NOT NULL,
881-
age INTEGER,
882-
weight_kg REAL,
885+
age INTEGER CHECK (age IS NULL OR (age >= 0 AND age <= 200)),
886+
weight_kg REAL CHECK (weight_kg IS NULL OR (weight_kg >= 0 AND weight_kg <= 700)),
883887
sex TEXT DEFAULT '',
884888
blood_type TEXT DEFAULT '',
885889
allergies TEXT DEFAULT '[]',
@@ -893,14 +897,14 @@ def _create_medical_security_tables(conn):
893897
CREATE TABLE IF NOT EXISTS vitals_log (
894898
id INTEGER PRIMARY KEY AUTOINCREMENT,
895899
patient_id INTEGER NOT NULL,
896-
bp_systolic INTEGER,
897-
bp_diastolic INTEGER,
898-
pulse INTEGER,
899-
resp_rate INTEGER,
900-
temp_f REAL,
901-
spo2 INTEGER,
902-
pain_level INTEGER,
903-
gcs INTEGER,
900+
bp_systolic INTEGER CHECK (bp_systolic IS NULL OR (bp_systolic >= 20 AND bp_systolic <= 350)),
901+
bp_diastolic INTEGER CHECK (bp_diastolic IS NULL OR (bp_diastolic >= 10 AND bp_diastolic <= 250)),
902+
pulse INTEGER CHECK (pulse IS NULL OR (pulse >= 0 AND pulse <= 300)),
903+
resp_rate INTEGER CHECK (resp_rate IS NULL OR (resp_rate >= 0 AND resp_rate <= 100)),
904+
temp_f REAL CHECK (temp_f IS NULL OR (temp_f >= 70.0 AND temp_f <= 115.0)),
905+
spo2 INTEGER CHECK (spo2 IS NULL OR (spo2 >= 0 AND spo2 <= 100)),
906+
pain_level INTEGER CHECK (pain_level IS NULL OR (pain_level >= 0 AND pain_level <= 10)),
907+
gcs INTEGER CHECK (gcs IS NULL OR (gcs >= 3 AND gcs <= 15)),
904908
notes TEXT DEFAULT '',
905909
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
906910
);

services/kiwix.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,8 @@ def start():
776776
log.warning('No ZIM files found — kiwix-serve needs content to run. Download a ZIM from the Library tab first.')
777777
raise RuntimeError('No content downloaded yet — add content from the Library tab before starting Kiwix')
778778

779-
args = ['--port', str(KIWIX_PORT), '--address', '0.0.0.0'] + zim_paths
779+
from config import Config
780+
args = ['--port', str(KIWIX_PORT), '--address', Config.APP_HOST] + zim_paths
780781
exe = get_exe_path()
781782

782783
from platform_utils import popen_kwargs

services/manager.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,20 @@ def get_services_dir():
5959
# ─── GPU Detection ────────────────────────────────────────────────────
6060

6161
_gpu_info = None
62+
_gpu_lock = threading.Lock()
6263

6364

6465
def detect_gpu() -> dict:
6566
"""Detect GPU type and capabilities (cross-platform)."""
66-
from platform_utils import detect_gpu as _detect_gpu
6767
global _gpu_info
6868
if _gpu_info is not None:
6969
return _gpu_info
70-
_gpu_info = _detect_gpu()
71-
return _gpu_info
70+
with _gpu_lock:
71+
if _gpu_info is not None:
72+
return _gpu_info
73+
from platform_utils import detect_gpu as _detect_gpu
74+
_gpu_info = _detect_gpu()
75+
return _gpu_info
7276

7377

7478
def get_ollama_gpu_env() -> dict:
@@ -220,6 +224,15 @@ def _read_output():
220224
except Exception as e:
221225
log.error(f'Failed to update DB for {service_id}: {e}')
222226
proc.terminate()
227+
try:
228+
proc.wait(timeout=5)
229+
except subprocess.TimeoutExpired:
230+
proc.kill()
231+
if proc.stdout:
232+
try:
233+
proc.stdout.close()
234+
except Exception:
235+
pass
223236
with _lock:
224237
_processes.pop(service_id, None)
225238
raise
@@ -284,8 +297,8 @@ def is_running(service_id: str) -> bool:
284297
"""
285298
with _lock:
286299
proc = _processes.get(service_id)
287-
if proc and proc.poll() is None:
288-
return True
300+
if proc and proc.poll() is None:
301+
return True
289302

290303
db = get_db()
291304
try:

services/ollama.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ def start():
184184

185185
from platform_utils import get_ollama_gpu_env
186186
env = get_ollama_gpu_env()
187-
env['OLLAMA_HOST'] = f'0.0.0.0:{OLLAMA_PORT}'
187+
from config import Config
188+
env['OLLAMA_HOST'] = f'{Config.APP_HOST}:{OLLAMA_PORT}'
188189
env['OLLAMA_MODELS'] = models_dir
189190

190191
from platform_utils import popen_kwargs

web/app.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,11 @@ def _check_mutating_rate_limit():
171171
return _j({'error': 'Rate limit exceeded for mutating requests',
172172
'retry_after': _mutating_window}), 429
173173
q.append(now)
174+
# Prune empty deques to prevent unbounded memory growth from old IPs
175+
if len(_mutating_hits) > 500:
176+
stale_addrs = [a for a, dq in _mutating_hits.items() if not dq]
177+
for a in stale_addrs:
178+
del _mutating_hits[a]
174179

175180
app.extensions['limiter'] = limiter
176181
except ImportError:

web/blueprints/federation.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ def api_node_sync_receive():
346346
log.warning('Federation sync insert failed for %s: %s', tname, e)
347347
imported[tname] = count
348348
total += count
349-
db.commit()
349+
# Do NOT commit yet — data + vector clocks must be atomic
350350

351351
incoming_clocks = data.get('vector_clocks', {})
352352
if not isinstance(incoming_clocks, dict):
@@ -389,11 +389,11 @@ def api_node_sync_receive():
389389
merged[node] = max(merged.get(node, 0), val)
390390
db.execute('INSERT OR REPLACE INTO vector_clocks (table_name, row_hash, clock, last_node, updated_at) VALUES (?, ?, ?, ?, datetime("now"))',
391391
(tname, row_hash, json.dumps(merged), source_node))
392-
db.commit()
393392

394393
db.execute('INSERT INTO sync_log (direction, peer_node_id, peer_name, peer_ip, tables_synced, items_count, status, conflicts_detected, conflict_details) VALUES (?,?,?,?,?,?,?,?,?)',
395394
('receive', source_node, source_name, request.remote_addr or '',
396395
json.dumps(imported), total, 'success', len(conflicts), json.dumps(conflicts, default=str)))
396+
# Single atomic commit: data + vector clocks + sync log
397397
db.commit()
398398

399399
log_activity('sync_received', detail=f'From {source_name} ({source_node}): {total} items')

web/blueprints/media.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,24 @@
2222
from web.sql_safety import safe_table, safe_columns, build_update
2323
import web.state as _state
2424
from web.utils import clone_json_fallback as _clone_json_fallback, safe_json_list as _safe_json_list, validate_download_url as _validate_download_url
25+
import posixpath as _posixpath
26+
27+
28+
def _sanitize_folder(raw):
29+
"""Sanitize a user-supplied folder path for media organization.
30+
31+
Collapses traversal sequences, rejects anything that escapes the root,
32+
and strips leading/trailing slashes. Returns a clean relative path or ''.
33+
"""
34+
if not raw:
35+
return ''
36+
# Normalize using posixpath to collapse .. and . regardless of OS
37+
normed = _posixpath.normpath(raw.replace('\\', '/'))
38+
# Reject any path that escapes root (starts with .. or is absolute)
39+
if normed.startswith('..') or normed.startswith('/'):
40+
return ''
41+
# Strip surrounding slashes and dots for safety
42+
return normed.strip('/').strip('.')
2543

2644
try:
2745
from web.catalog import CHANNEL_CATALOG, CHANNEL_CATEGORIES
@@ -409,7 +427,7 @@ def api_videos_upload():
409427
file.save(filepath)
410428
filesize = os.path.getsize(filepath) if os.path.isfile(filepath) else 0
411429
category = request.form.get('category', 'general')
412-
folder = request.form.get('folder', '').replace('..', '').strip('/')
430+
folder = _sanitize_folder(request.form.get('folder', ''))
413431
title = request.form.get('title', filename.rsplit('.', 1)[0])
414432
with db_session() as db:
415433
cur = db.execute('INSERT INTO videos (title, filename, category, folder, filesize) VALUES (?, ?, ?, ?, ?)',
@@ -1502,7 +1520,7 @@ def api_audio_upload():
15021520
filesize = os.path.getsize(filepath) if os.path.isfile(filepath) else 0
15031521
title = request.form.get('title', filename.rsplit('.', 1)[0])
15041522
category = request.form.get('category', 'general')
1505-
folder = request.form.get('folder', '').replace('..', '').strip('/')
1523+
folder = _sanitize_folder(request.form.get('folder', ''))
15061524
artist = request.form.get('artist', '')
15071525
with db_session() as db:
15081526
cur = db.execute('INSERT INTO audio (title, filename, category, folder, artist, filesize) VALUES (?, ?, ?, ?, ?, ?)',
@@ -2091,7 +2109,7 @@ def api_books_upload():
20912109
title = request.form.get('title', filename.rsplit('.', 1)[0])
20922110
author = request.form.get('author', '')
20932111
category = request.form.get('category', 'general')
2094-
folder = request.form.get('folder', '').replace('..', '').strip('/')
2112+
folder = _sanitize_folder(request.form.get('folder', ''))
20952113
with db_session() as db:
20962114
cur = db.execute('INSERT INTO books (title, author, filename, format, category, folder, filesize) VALUES (?, ?, ?, ?, ?, ?, ?)',
20972115
(title, author, filename, fmt, category, folder, filesize))

web/blueprints/situation_room.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -660,8 +660,10 @@ def _fetch_rss_feeds():
660660
for fut in as_completed(futures, timeout=90):
661661
try:
662662
articles.extend(fut.result())
663-
except Exception:
664-
pass
663+
except Exception as e:
664+
feed = futures.get(fut)
665+
feed_name = feed.get('name', '?') if feed else '?'
666+
log.debug('RSS fetch worker failed for %s: %s', feed_name, e)
665667

666668
if not articles:
667669
return

web/blueprints/system.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1872,7 +1872,7 @@ def api_system_db_vacuum():
18721872
"""Run VACUUM and REINDEX to optimize the database."""
18731873
import sqlite3
18741874
path = get_db_path()
1875-
conn = sqlite3.connect(path)
1875+
conn = sqlite3.connect(path, uri=path.startswith('file:'))
18761876
conn.execute('VACUUM')
18771877
conn.execute('REINDEX')
18781878
conn.close()

0 commit comments

Comments
 (0)