From 5e695b7c39c03ec3887b8a238f41ef9e0df82ccf Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:22:24 -0400 Subject: [PATCH] fix(web): keep exception text out of calendar responses; annotate moved code The split made scanners report existing findings in the moved code as new: - CodeQL: the calendar auth and calendar-list routes returned exception text (redacted, but still derived from the exception). Both now log the exception and return a fixed message pointing at the log. - MD5 in the asset upload only makes a filename unique: usedforsecurity=False. - pickle reads/writes the calendar plugin's own OAuth token (as before): annotated. Token-status labels and a log line naming the secrets path are false positives: annotated with the repo's nosec/nosemgrep convention. Co-Authored-By: Claude Opus 5.5 --- .../test_calendar_oauth_endpoints.py | 7 ++++--- web_interface/blueprints/api_v3/__init__.py | 5 +++-- .../blueprints/api_v3/plugin_assets.py | 4 +++- .../blueprints/api_v3/plugin_calendar.py | 17 ++++++++--------- .../blueprints/api_v3/plugin_config.py | 3 ++- web_interface/blueprints/api_v3/plugin_store.py | 6 +++--- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/test/web_interface/test_calendar_oauth_endpoints.py b/test/web_interface/test_calendar_oauth_endpoints.py index b6e86943..a281be99 100644 --- a/test/web_interface/test_calendar_oauth_endpoints.py +++ b/test/web_interface/test_calendar_oauth_endpoints.py @@ -402,8 +402,8 @@ class TestDiagnosticsAreRedacted: tmp_path, monkeypatch): # OSError from the spawn carries the interpreter path and whatever the - # OS chose to say; it reaches the client through the redactor like - # everything else. + # OS chose to say; none of it reaches the client, which is pointed at + # the log instead. script = tmp_path / 'calendar_registration.py' script.write_text('', encoding='utf-8') @@ -414,7 +414,8 @@ class TestDiagnosticsAreRedacted: payload, error = mod._run_calendar_registration(tmp_path, '') assert payload is None assert 'abcd1234' not in error, error - assert 'OSError' in error, error + assert 'Exec format' not in error, error + assert 'log' in error, error def test_a_missing_google_library_is_reported_without_raw_exception_text( self, client, monkeypatch): diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index 6009ecba..759322b5 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -1466,8 +1466,9 @@ def _run_calendar_registration(plugin_dir: Path, stdin_payload: str): except subprocess.TimeoutExpired: return None, 'Authentication timed out after 120s' except OSError as e: - logger.error('Could not run calendar_registration.py', exc_info=True) - return None, 'Could not run the authentication script: %s' % describe_exception(e) + # The exception (a path, an errno) goes to the log, not the client. + logger.error('Could not run calendar_registration.py: %s', e, exc_info=True) + return None, 'Could not run the authentication script; see the web service log' for line in reversed((result.stdout or '').splitlines()): line = line.strip() diff --git a/web_interface/blueprints/api_v3/plugin_assets.py b/web_interface/blueprints/api_v3/plugin_assets.py index 58811ef0..58522117 100644 --- a/web_interface/blueprints/api_v3/plugin_assets.py +++ b/web_interface/blueprints/api_v3/plugin_assets.py @@ -142,7 +142,9 @@ def upload_plugin_asset(): for file, file_ext, file_size, file_content in accepted: # Generate unique filename timestamp = int(_pkg.time.time()) - file_hash = hashlib.md5(file_content + file.filename.encode()).hexdigest()[:8] + # Only makes the filename unique; nothing is verified with it. + file_hash = hashlib.md5(file_content + file.filename.encode(), + usedforsecurity=False).hexdigest()[:8] safe_filename = f"image_{timestamp}_{file_hash}{file_ext}" file_path = assets_dir / safe_filename diff --git a/web_interface/blueprints/api_v3/plugin_calendar.py b/web_interface/blueprints/api_v3/plugin_calendar.py index 70e25ff0..5c871dd3 100644 --- a/web_interface/blueprints/api_v3/plugin_calendar.py +++ b/web_interface/blueprints/api_v3/plugin_calendar.py @@ -6,7 +6,7 @@ so their endpoint names do not depend on which module they live in. from web_interface.blueprints.api_v3 import ( Path, _CALENDAR_LIST_MAX_PAGES, _plugin_directory, _prune_credential_backups, _run_calendar_registration, api_v3, - describe_exception, json, jsonify, logger, os, redact_text, request, + json, jsonify, logger, os, redact_text, request, shutil, ) from src.config_manager_atomic import atomic_write_text @@ -158,29 +158,28 @@ def list_calendar_calendars(): }), 400 try: - import pickle + import pickle # nosec B403 - reads only the plugin's own OAuth token # nosemgrep from google.auth.transport.requests import Request as GoogleRequest from googleapiclient.discovery import build as build_google_service except ImportError as e: + # Which module is missing goes to the log; the exception itself (it + # can quote a path) doesn't go to the client. + logger.warning('Calendar picker: Google API libraries missing: %s', e) return jsonify({ 'status': 'error', - # The name of the missing module is the whole diagnosis, but it - # arrives as an exception, so it goes through the redactor like - # any other -- an ImportError can quote a path. 'message': ('The Google API libraries are not installed. Install ' - "the calendar plugin's requirements.txt. (%s)" - % describe_exception(e)) + "the calendar plugin's requirements.txt.") }), 500 with open(token_file, 'rb') as handle: # Written only by this plugin's own OAuth flow, into its own # directory, and read here exactly as the plugin itself reads it. - creds = pickle.load(handle) # nosec B301 - locally generated token + creds = pickle.load(handle) # nosec B301 - locally generated token # nosemgrep if creds and creds.expired and creds.refresh_token: creds.refresh(GoogleRequest()) with open(token_file, 'wb') as handle: - pickle.dump(creds, handle) + pickle.dump(creds, handle) # nosec B301 - same token, same format # nosemgrep os.chmod(token_file, 0o600) if not creds or not creds.valid: diff --git a/web_interface/blueprints/api_v3/plugin_config.py b/web_interface/blueprints/api_v3/plugin_config.py index 1f6e4bbe..7aed9110 100644 --- a/web_interface/blueprints/api_v3/plugin_config.py +++ b/web_interface/blueprints/api_v3/plugin_config.py @@ -435,7 +435,8 @@ def save_plugin_config(): ) except Exception: secrets_path = api_v3.config_manager.secrets_path - logger.error("Error saving secrets config for %s (path=%s)", plugin_id, secrets_path, exc_info=True) + # Logs the file path, not any secret value. + logger.error("Error saving secrets config for %s (path=%s)", plugin_id, secrets_path, exc_info=True) # nosemgrep return error_response( ErrorCode.CONFIG_SAVE_FAILED, "Failed to save secrets configuration; see logs for details", diff --git a/web_interface/blueprints/api_v3/plugin_store.py b/web_interface/blueprints/api_v3/plugin_store.py index 82ca04d5..c4603258 100644 --- a/web_interface/blueprints/api_v3/plugin_store.py +++ b/web_interface/blueprints/api_v3/plugin_store.py @@ -738,7 +738,7 @@ def get_github_auth_status(): return jsonify({ 'status': 'success', 'data': { - 'token_status': 'none', + 'token_status': 'none', # nosec B105 - a status label # nosemgrep 'authenticated': False, 'rate_limit': 60, 'message': 'No GitHub token configured', @@ -753,7 +753,7 @@ def get_github_auth_status(): return jsonify({ 'status': 'success', 'data': { - 'token_status': 'valid', + 'token_status': 'valid', # nosec B105 - a status label # nosemgrep 'authenticated': True, 'rate_limit': 5000, 'message': 'GitHub API authenticated', @@ -764,7 +764,7 @@ def get_github_auth_status(): return jsonify({ 'status': 'success', 'data': { - 'token_status': 'invalid', + 'token_status': 'invalid', # nosec B105 - a status label # nosemgrep 'authenticated': False, 'rate_limit': 60, 'message': f'GitHub token is invalid: {error_message}' if error_message else 'GitHub token is invalid',