dandri/meshcore-bot
1
0
Fork 0
mirror of https://github.com/agessaman/meshcore-bot.git synced 2026-08-28 05:14:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enhance database management and validation in DBManager and BotDataViewer

Browse Source 
- Introduced validation for SQLite journal modes in DBManager, defaulting to "WAL" for invalid inputs and logging warnings.
- Added a new utility function in security_utils for validating SQL identifiers to prevent SQL injection.
- Updated BotDataViewer to utilize the new journal mode validation and SQL identifier checks, ensuring safer database operations.
- Enhanced test coverage for restore functionality, including checks for backup directory configuration and path traversal prevention.
This commit is contained in:
agessaman
2026-03-29 09:45:18 -07:00
parent fa4d0c200c
commit d0497d983e
4 changed files with 121 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/db_manager.py
+4
View File
@@ -12,6 +12,7 @@ from contextlib import asynccontextmanager, contextmanager
from typing import Any, Optional
from .db_migrations import MigrationRunner
from .security_utils import VALID_JOURNAL_MODES
class DBManager:
@@ -478,6 +479,9 @@ class DBManager:
busy_timeout_ms = default_busy_timeout_ms
foreign_keys = bool(foreign_keys)
journal_mode = str(journal_mode).strip() or "WAL"
if journal_mode.upper() not in VALID_JOURNAL_MODES:
self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL")
journal_mode = "WAL"
try:
conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}")
modules/security_utils.py
+26
View File
@@ -243,6 +243,32 @@ def sanitize_input(content: str, max_length: Optional[int] = 500, strip_controls
return content.strip()
# Valid SQLite journal modes for PRAGMA journal_mode validation
VALID_JOURNAL_MODES = {"DELETE", "TRUNCATE", "PERSIST", "MEMORY", "WAL", "OFF"}
def validate_sql_identifier(identifier: str) -> str:
"""
Validate a SQL identifier (table or column name) for safe interpolation.
Only allows alphanumeric characters and underscores, must start with
a letter or underscore. This is intentionally strict to prevent SQL injection
when parameterized queries cannot be used (e.g., PRAGMA, REINDEX).
Args:
identifier: The SQL identifier to validate
Returns:
The validated identifier string
Raises:
ValueError: If the identifier contains unsafe characters
"""
if not re.match(r'^[a-zA-Z_][a-zA-Z0-9_]*$', identifier):
raise ValueError(f"Invalid SQL identifier: {identifier!r}")
return identifier
def validate_api_key_format(api_key: str, min_length: int = 16) -> bool:
"""
Validate API key format
modules/web_viewer/app.py
+55 -11
View File
@@ -32,6 +32,8 @@ from flask import (
)
from flask_socketio import SocketIO, disconnect, emit
from modules.security_utils import VALID_JOURNAL_MODES, validate_sql_identifier
def _apply_werkzeug_websocket_fix() -> None:
"""Patch SimpleWebSocketWSGI to call start_response after WebSocket teardown.
@@ -99,16 +101,16 @@ class BotDataViewer:
self.app.config['SECRET_KEY'] = _secrets.token_hex(32)
# Flask-SocketIO configuration following 5.x best practices
self.socketio = SocketIO(
self.app,
cors_allowed_origins="*",
# CORS origins are configured after config is loaded; create without app for now
self._socketio_kwargs = dict(
max_http_buffer_size=1000000, # 1MB buffer limit
ping_timeout=20, # 20 second ping timeout — 5s was too short when subscribe handlers replay DB history
ping_interval=25, # 25 second ping interval (Flask-SocketIO 5.x default)
logger=False, # Disable verbose logging
engineio_logger=False, # Disable EngineIO logging
async_mode='threading' # Use threading for better stability
async_mode='threading', # Use threading for better stability
)
self.socketio = SocketIO()
self.repeater_db_path = repeater_db_path
@@ -153,6 +155,14 @@ class BotDataViewer:
"or restrict access with host = 127.0.0.1 and firewall rules."
)
# Configure CORS for SocketIO — default to same-origin (no cross-origin)
cors_raw = self.config.get('Web_Viewer', 'cors_allowed_origins', fallback='').strip()
if cors_raw:
cors_origins = cors_raw if cors_raw == '*' else [o.strip() for o in cors_raw.split(',') if o.strip()]
self._socketio_kwargs['cors_allowed_origins'] = cors_origins
# Initialize SocketIO with Flask app now that config is loaded
self.socketio.init_app(self.app, **self._socketio_kwargs)
# Version info for footer (tag or branch/commit/date); computed once at startup
self._version_info = self._get_version_info()
@@ -347,6 +357,9 @@ class BotDataViewer:
foreign_keys = self.config.getboolean("Web_Viewer", "sqlite_foreign_keys", fallback=True)
busy_timeout_ms = self.config.getint("Web_Viewer", "sqlite_busy_timeout_ms", fallback=60000)
journal_mode = self.config.get("Web_Viewer", "sqlite_journal_mode", fallback="WAL").strip() or "WAL"
if journal_mode.upper() not in VALID_JOURNAL_MODES:
self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL")
journal_mode = "WAL"
conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}")
conn.execute(f"PRAGMA busy_timeout={int(busy_timeout_ms)}")
conn.execute(f"PRAGMA journal_mode={journal_mode}")
@@ -368,6 +381,9 @@ class BotDataViewer:
foreign_keys = self.config.getboolean("Web_Viewer", "sqlite_foreign_keys", fallback=True)
busy_timeout_ms = self.config.getint("Web_Viewer", "sqlite_busy_timeout_ms", fallback=60000)
journal_mode = self.config.get("Web_Viewer", "sqlite_journal_mode", fallback="WAL").strip() or "WAL"
if journal_mode.upper() not in VALID_JOURNAL_MODES:
self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL")
journal_mode = "WAL"
conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}")
conn.execute(f"PRAGMA busy_timeout={int(busy_timeout_ms)}")
conn.execute(f"PRAGMA journal_mode={journal_mode}")
@@ -1471,7 +1487,19 @@ class BotDataViewer:
db_file = str(data.get('db_file', '')).strip()
if not db_file:
return jsonify({'error': 'db_file is required'}), 400
src = Path(db_file)
# Validate path is within the configured backup directory
backup_dir_str = self.db_manager.get_metadata('maint.db_backup_dir') or ''
if not backup_dir_str or not os.path.isdir(backup_dir_str):
return jsonify({'error': 'No valid backup directory configured'}), 400
# Ensure the resolved path is within the backup directory (prevents traversal)
backup_dir = Path(backup_dir_str).resolve()
src = Path(db_file).resolve()
try:
src.relative_to(backup_dir)
except ValueError:
return jsonify({'error': 'Restore path must be within the configured backup directory'}), 403
if not src.exists():
return jsonify({'error': f'File not found: {db_file}'}), 400
# Validate it is a real SQLite file by checking the magic header
@@ -4094,6 +4122,11 @@ class BotDataViewer:
stats['active_cache_entries'] = 0
for table in cache_tables:
try:
validate_sql_identifier(table)
except ValueError:
self.logger.warning(f"Skipping invalid table name: {table!r}")
continue
cursor.execute(f"SELECT COUNT(*) FROM {table}")
count = cursor.fetchone()[0]
stats['total_cache_entries'] += count
@@ -4373,6 +4406,11 @@ class BotDataViewer:
total_records = 0
for table_name in table_names:
try:
validate_sql_identifier(table_name)
except ValueError:
self.logger.warning(f"Skipping invalid table name: {table_name!r}")
continue
try:
# Get record count
cursor.execute(f"SELECT COUNT(*) FROM {table_name}")
@@ -4483,12 +4521,18 @@ class BotDataViewer:
self.logger.info("Starting database REINDEX...")
reindexed_tables = []
for table in tables:
if table != 'sqlite_sequence': # Skip system tables
try:
cursor.execute(f"REINDEX {table}")
reindexed_tables.append(table)
except Exception as e:
self.logger.debug(f"Could not reindex table {table}: {e}")
if table == 'sqlite_sequence': # Skip system tables
continue
try:
validate_sql_identifier(table)
except ValueError:
self.logger.warning(f"Skipping invalid table name for REINDEX: {table!r}")
continue
try:
cursor.execute(f"REINDEX {table}")
reindexed_tables.append(table)
except Exception as e:
self.logger.debug(f"Could not reindex table {table}: {e}")
# Get final database size
final_size = os.path.getsize(self.db_path)
tests/test_web_viewer.py
+36 -4
View File
@@ -1911,18 +1911,47 @@ class TestRestoreRoute:
assert resp.status_code == 400
assert "db_file" in resp.get_json()["error"]
def test_nonexistent_file_returns_400(self, viewer):
def test_nonexistent_file_returns_400(self, viewer, tmp_path):
"""Returns 400 when db_file path does not exist."""
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
viewer.db_manager.set_metadata('maint.db_backup_dir', str(backup_dir))
with viewer.app.test_client() as c:
resp = c.post("/api/maintenance/restore",
json={"db_file": "/no/such/file.db"},
json={"db_file": str(backup_dir / "missing.db")},
content_type="application/json")
assert resp.status_code == 400
assert "not found" in resp.get_json()["error"].lower()
def test_path_traversal_returns_403(self, viewer, tmp_path):
"""Returns 403 when db_file is outside the backup directory."""
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
viewer.db_manager.set_metadata('maint.db_backup_dir', str(backup_dir))
outside = tmp_path / "outside.db"
outside.write_bytes(b"SQLite format 3\x00")
with viewer.app.test_client() as c:
resp = c.post("/api/maintenance/restore",
json={"db_file": str(outside)},
content_type="application/json")
assert resp.status_code == 403
def test_no_backup_dir_returns_400(self, viewer):
"""Returns 400 when no backup directory is configured."""
viewer.db_manager.set_metadata('maint.db_backup_dir', '')
with viewer.app.test_client() as c:
resp = c.post("/api/maintenance/restore",
json={"db_file": "/some/file.db"},
content_type="application/json")
assert resp.status_code == 400
assert "backup directory" in resp.get_json()["error"].lower()
def test_non_sqlite_file_returns_400(self, viewer, tmp_path):
"""Returns 400 when the file is not a valid SQLite database."""
bad = tmp_path / "bad.db"
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
viewer.db_manager.set_metadata('maint.db_backup_dir', str(backup_dir))
bad = backup_dir / "bad.db"
bad.write_bytes(b"not a sqlite file!!")
with viewer.app.test_client() as c:
resp = c.post("/api/maintenance/restore",
@@ -1934,7 +1963,10 @@ class TestRestoreRoute:
def test_valid_sqlite_restore_returns_200(self, viewer, tmp_path):
"""Returns 200 with warning when a valid SQLite backup is restored."""
import sqlite3 as _sql
backup = tmp_path / "backup.db"
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
viewer.db_manager.set_metadata('maint.db_backup_dir', str(backup_dir))
backup = backup_dir / "backup.db"
conn = _sql.connect(str(backup))
conn.execute("CREATE TABLE t (id INTEGER)")
conn.commit()