From 0bd9f129aedfc3fa8fdbb7ae392ea5ed5dbcd8e3 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Thu, 26 Feb 2026 18:20:52 -0500 Subject: [PATCH] Hardened `get_user_settings()` to normalize malformed or missing `settings` to `{}` and persist the repaired document. --- application/single_app/config.py | 2 +- application/single_app/functions_settings.py | 11 +- .../single_app/route_frontend_chats.py | 11 +- .../CHATS_USER_SETTINGS_HARDENING_FIX.md | 37 +++++ docs/explanation/release_notes.md | 13 ++ .../test_chats_user_settings_hardening_fix.py | 127 ++++++++++++++++++ 6 files changed, 193 insertions(+), 8 deletions(-) create mode 100644 docs/explanation/fixes/CHATS_USER_SETTINGS_HARDENING_FIX.md create mode 100644 functional_tests/test_chats_user_settings_hardening_fix.py diff --git a/application/single_app/config.py b/application/single_app/config.py index a6a3bc99..7094a3c5 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -88,7 +88,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.238.024" +VERSION = "0.238.025" SECRET_KEY = os.getenv('SECRET_KEY', 'dev-secret-key-change-in-production') diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py index f7325585..8176939d 100644 --- a/application/single_app/functions_settings.py +++ b/application/single_app/functions_settings.py @@ -551,15 +551,22 @@ def get_user_settings(user_id): from flask import session try: doc = cosmos_user_settings_container.read_item(item=user_id, partition_key=user_id) + updated = False + # Ensure the settings key exists for consistency downstream - if 'settings' not in doc: + if 'settings' not in doc or not isinstance(doc.get('settings'), dict): + previous_type = type(doc.get('settings')).__name__ if 'settings' in doc else 'missing' doc['settings'] = {} + updated = True + log_event("[UserSettings] Malformed settings repaired", { + "user_id": user_id, + "previous_type": previous_type, + }) # Try to update email/display_name if missing and available in session user = session.get("user", {}) email = user.get("preferred_username") or user.get("email") display_name = user.get("name") - updated = False if email and doc.get("email") != email: doc["email"] = email updated = True diff --git a/application/single_app/route_frontend_chats.py b/application/single_app/route_frontend_chats.py index 61a2899e..ca0feb1a 100644 --- a/application/single_app/route_frontend_chats.py +++ b/application/single_app/route_frontend_chats.py @@ -21,14 +21,18 @@ def register_route_frontend_chats(app): @user_required def chats(): user_id = get_current_user_id() + if not user_id: + return redirect(url_for('login')) + settings = get_settings() user_settings = get_user_settings(user_id) + user_settings_dict = user_settings.get("settings", {}) if isinstance(user_settings, dict) else {} public_settings = sanitize_settings_for_user(settings) enable_user_feedback = public_settings.get("enable_user_feedback", False) enable_enhanced_citations = public_settings.get("enable_enhanced_citations", False) enable_document_classification = public_settings.get("enable_document_classification", False) enable_extract_meta_data = public_settings.get("enable_extract_meta_data", False) - active_group_id = user_settings["settings"].get("activeGroupOid", "") + active_group_id = user_settings_dict.get("activeGroupOid", "") active_group_name = "" if active_group_id: group_doc = find_group_by_id(active_group_id) @@ -36,13 +40,10 @@ def chats(): active_group_name = group_doc.get("name", "") # Get active public workspace ID from user settings - active_public_workspace_id = user_settings["settings"].get("activePublicWorkspaceOid", "") + active_public_workspace_id = user_settings_dict.get("activePublicWorkspaceOid", "") categories_list = public_settings.get("document_classification_categories","") - if not user_id: - return redirect(url_for('login')) - # Get user display name from user settings user_display_name = user_settings.get('display_name', '') diff --git a/docs/explanation/fixes/CHATS_USER_SETTINGS_HARDENING_FIX.md b/docs/explanation/fixes/CHATS_USER_SETTINGS_HARDENING_FIX.md new file mode 100644 index 00000000..abef15cd --- /dev/null +++ b/docs/explanation/fixes/CHATS_USER_SETTINGS_HARDENING_FIX.md @@ -0,0 +1,37 @@ +# Chats User Settings Hardening Fix (v0.238.025) + +## Issue Description +A single user could fail to load the chats page while other users worked normally. + +## Root Cause Analysis +The chats route expected `user_settings["settings"]` to always be a dictionary. If that field existed but had malformed data (for example, string/null/list), the route could throw before rendering. + +## Version Implemented +Fixed/Implemented in version: **0.238.025** + +## Technical Details +### Files Modified +- application/single_app/functions_settings.py +- application/single_app/route_frontend_chats.py +- application/single_app/config.py +- functional_tests/test_chats_user_settings_hardening_fix.py + +### Code Changes Summary +- Hardened `get_user_settings()` to normalize malformed or missing `settings` to `{}` and persist the repaired document. +- Added repair telemetry for malformed settings shape. +- Hardened `/chats` route to safely read nested settings with dictionary fallbacks. +- Incremented application version. + +### Testing Approach +- Added a functional regression test validating both malformed and missing `settings` cases are repaired and upserted. + +## Impact Analysis +- Healthy users are unaffected. +- Corrupted user settings documents are self-healed on read. +- Prevents user-specific chats page crashes caused by malformed settings shape. + +## Validation +- Functional test: functional_tests/test_chats_user_settings_hardening_fix.py + +## Reference to Config Version Update +- Version updated in application/single_app/config.py to **0.238.025**. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index c474eb28..cb4841b5 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,19 @@ # Feature Release +### **(v0.238.025)** + +#### Bug Fixes + +* **Chats Page User Settings Hardening** + * Fixed a user-specific chats page failure where only one affected user could not load `/chats` due to malformed per-user settings data. + * **Root Cause**: The chats route assumed `user_settings["settings"]` was always a dictionary. If that field existed but had an invalid type (for example string, null, or list), the page could fail before rendering. + * **Solution**: Hardened `get_user_settings()` to normalize missing/malformed `settings` to `{}` and persist the repaired document. Hardened the chats route to use safe dictionary fallbacks when reading nested settings values. + * **Telemetry**: Added repair logging (`[UserSettings] Malformed settings repaired`) to improve diagnostics for future user-specific data-shape issues. + * **Files Modified**: `functions_settings.py`, `route_frontend_chats.py`, `config.py`. + * **Files Added**: `test_chats_user_settings_hardening_fix.py`, `CHATS_USER_SETTINGS_HARDENING_FIX.md`. + * (Ref: user settings normalization, `/chats` route resilience, `functional_tests/test_chats_user_settings_hardening_fix.py`, `docs/explanation/fixes/CHATS_USER_SETTINGS_HARDENING_FIX.md`) + ### **(v0.238.024)** #### New Features diff --git a/functional_tests/test_chats_user_settings_hardening_fix.py b/functional_tests/test_chats_user_settings_hardening_fix.py new file mode 100644 index 00000000..be3346d4 --- /dev/null +++ b/functional_tests/test_chats_user_settings_hardening_fix.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +""" +Functional test for chats user settings hardening fix. +Version: 0.238.025 +Implemented in: 0.238.025 + +This test ensures that malformed user settings documents are safely normalized +and do not crash the /chats page path that reads nested settings values. +""" + +# pyright: reportMissingImports=false + +import os +import sys +import types + +from flask import Flask, session + + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', 'application', 'single_app')) + + +def _install_auth_stub(): + sys.modules['functions_authentication'] = types.SimpleNamespace( + get_user_profile_image=lambda: None + ) + + +class FakeContainer: + def __init__(self, doc): + self.doc = doc + self.upsert_calls = [] + + def read_item(self, item, partition_key): + return self.doc + + def upsert_item(self, body): + self.doc = body + self.upsert_calls.append(body) + + +def test_get_user_settings_repairs_non_dict_settings(): + print("๐Ÿ” Testing non-dict settings repair...") + + import functions_settings + + _install_auth_stub() + + fake_doc = { + 'id': 'user-1', + 'settings': 'corrupted-value', + 'display_name': 'Existing Name', + } + fake_container = FakeContainer(fake_doc) + functions_settings.cosmos_user_settings_container = fake_container + + app = Flask(__name__) + app.secret_key = 'test-secret' + + with app.test_request_context('/'): + session['user'] = { + 'preferred_username': 'user1@example.com', + 'name': 'User One', + } + + result = functions_settings.get_user_settings('user-1') + + assert isinstance(result.get('settings'), dict), 'settings should be normalized to a dict' + assert result['settings'].get('profileImage') is None, 'profileImage should be set to None by stub' + assert len(fake_container.upsert_calls) >= 1, 'repaired document should be upserted' + + print("โœ… Non-dict settings are repaired and persisted") + return True + + +def test_get_user_settings_repairs_missing_settings_key(): + print("๐Ÿ” Testing missing settings key repair...") + + import functions_settings + + _install_auth_stub() + + fake_doc = { + 'id': 'user-2', + 'display_name': 'User Two', + } + fake_container = FakeContainer(fake_doc) + functions_settings.cosmos_user_settings_container = fake_container + + app = Flask(__name__) + app.secret_key = 'test-secret' + + with app.test_request_context('/'): + session['user'] = { + 'preferred_username': 'user2@example.com', + 'name': 'User Two', + } + + result = functions_settings.get_user_settings('user-2') + + assert isinstance(result.get('settings'), dict), 'missing settings key should be initialized as dict' + assert len(fake_container.upsert_calls) >= 1, 'initialized document should be upserted' + + print("โœ… Missing settings key is initialized and persisted") + return True + + +if __name__ == '__main__': + tests = [ + test_get_user_settings_repairs_non_dict_settings, + test_get_user_settings_repairs_missing_settings_key, + ] + + results = [] + for test in tests: + print(f"\n๐Ÿงช Running {test.__name__}...") + try: + results.append(test()) + except Exception as e: + print(f"โŒ {test.__name__} failed: {e}") + import traceback + traceback.print_exc() + results.append(False) + + success = all(results) + print(f"\n๐Ÿ“Š Results: {sum(results)}/{len(results)} tests passed") + sys.exit(0 if success else 1)