Skip to content

Commit 30dd7b0

Browse files
committed
Gemini, Copilot, Sonarqube review and fix
1 parent 2ce4450 commit 30dd7b0

6 files changed

Lines changed: 95 additions & 75 deletions

File tree

app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,6 @@ def listen_for_index_reloads():
761761
# --- Blueprint Registration ---
762762
# Standard Flask factory pattern: blueprint imports are inside
763763
# this function so the eager import graph stays flat.
764-
from app_cron import run_due_cron_jobs
765764

766765

767766
def _register_blueprints(flask_app):
@@ -905,6 +904,7 @@ def _start_map_init_background():
905904
def _cron_manager_loop():
906905
try:
907906
from time import sleep
907+
from app_cron import run_due_cron_jobs
908908
while True:
909909
try:
910910
with app.app_context():

app_cron.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,6 @@ def run_due_cron_jobs():
240240
"pca_components_max": int(PCA_COMPONENTS_MAX),
241241
"num_clustering_runs": int(CLUSTERING_RUNS),
242242
"max_songs_per_cluster_val": int(MAX_SONGS_PER_CLUSTER),
243-
"gmm_n_components_min": int(GMM_N_COMPONENTS_MIN),
244-
"gmm_n_components_max": int(GMM_N_COMPONENTS_MAX),
245243
"top_n_playlists_param": int(TOP_N_PLAYLISTS),
246244
"min_songs_per_genre_for_stratification_param": int(MIN_SONGS_PER_GENRE_FOR_STRATIFICATION),
247245
"stratified_sampling_target_percentile_param": int(STRATIFIED_SAMPLING_TARGET_PERCENTILE),

app_logging.py

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,17 @@
1111
``logger.info(...)`` from task modules fell through to Python's ``lastResort``
1212
handler — silently dropping INFO-level output during long-running jobs.
1313
14-
Emoji safety: the ``EmojiStrippingFilter`` removes emoji and other non-Latin-1
15-
symbols from every log record before it reaches a handler. This prevents
16-
``UnicodeEncodeError`` / ``UnicodeDecodeError`` crashes on Windows when stdout
17-
is a pipe (PyInstaller native build) or when the console code-page cannot
18-
represent the character. HTML templates and web-UI progress messages are
19-
unaffected — only the Python ``logging`` pipeline is sanitised.
14+
Record sanitization: the ``LogSanitizingFilter`` cleans every log record before
15+
it reaches a handler. It removes emoji / non-Latin-1 symbols (which raise
16+
``UnicodeEncodeError`` / ``UnicodeDecodeError`` on Windows when stdout is a pipe
17+
or the console code-page cannot represent the character) and neutralizes CR/LF
18+
and other control characters so an attacker-controlled value embedded in a
19+
message cannot forge or split log lines (CWE-117, log injection). This is the
20+
single, centralized place where log-message sanitization happens — call sites
21+
log the raw value and the filter cleans it. HTML templates and web-UI progress
22+
messages are unaffected — only the Python ``logging`` pipeline is sanitised, and
23+
the traceback ``logger.exception`` appends is left intact (the formatter renders
24+
it after filtering, so the full error is always visible in the log).
2025
"""
2126

2227
import logging
@@ -26,55 +31,69 @@
2631
LOG_DATEFMT = "%d-%m-%Y %H-%M-%S"
2732

2833
# ---------------------------------------------------------------------------
29-
# Emoji / symbol stripping for console-safe logging
34+
# Console-safe + injection-safe log record sanitization
3035
# ---------------------------------------------------------------------------
3136
# Ranges cover all common emoji blocks plus Dingbats, Misc Symbols,
3237
# Geometric Shapes, Supplemental Symbols, and variation selectors.
33-
# Characters within Latin-1 (U+0000U+00FF) are *not* stripped, so
34-
# European accented letters (e.g. é, ñ, ü) pass through unchanged.
38+
# Characters within Latin-1 (U+0000-U+00FF) are *not* stripped, so
39+
# European accented letters pass through unchanged.
3540
_EMOJI_RE = re.compile(
3641
"[\U0001F300-\U0001F9FF" # Misc Symbols, Emoticons, Transport, Supplemental
3742
"\U0001FA00-\U0001FAFF" # Chess Symbols, Symbols Extended-A
38-
"\U00002190-\U000027BF" # Arrows (→ ← ↑ ↓ ↔), Misc Technical, Dingbats (✓ ✗ ✕ ★ ☆ ♯ ♭ etc.)
39-
"\U000025A0-\U000025FF" # Geometric Shapes (● ○ ■ □ ◆ ◇ ▲ ▼ etc.)
43+
"\U00002190-\U000027BF" # Arrows, Misc Technical, Dingbats
44+
"\U000025A0-\U000025FF" # Geometric Shapes
4045
"\U00002B00-\U00002BFF" # Misc Symbols & Arrows
4146
"\U0001F000-\U0001F02F" # Mahjong Tiles
4247
"\U0001F0A0-\U0001F0FF" # Playing Cards
43-
"\uFE0F\u200D" # Variation Selector-16, Zero-Width Joiner
48+
"\\uFE0F\\u200D" # Variation Selector-16, Zero-Width Joiner
4449
"]+"
4550
)
4651

52+
# C0 control codes plus DEL, EXCEPT tab (0x09). Includes LF (0x0A) and CR (0x0D):
53+
# replacing these with a space prevents a value with embedded newlines from
54+
# forging additional log lines.
55+
_CONTROL_RE = re.compile(r"[\x00-\x08\x0A-\x1F\x7F]")
4756

48-
def _strip_emoji(text: str) -> str:
49-
"""Remove emoji and symbol characters from *text*, returning a plain string."""
57+
58+
def _sanitize_log_text(text: str) -> str:
59+
"""Make *text* safe for a single console log line.
60+
61+
Strips emoji/symbol characters (Windows code-page safety) and replaces CR/LF
62+
and other C0 control codes with a space so an attacker-controlled value
63+
cannot forge or split log lines (CWE-117). Tabs are preserved.
64+
"""
5065
if not isinstance(text, str):
5166
return text
5267
cleaned = _EMOJI_RE.sub("", text)
53-
# Collapse multiple spaces that may result from removing a symbol
68+
cleaned = _CONTROL_RE.sub(" ", cleaned)
69+
# Collapse runs of spaces left behind by removed symbols / control codes.
5470
return re.sub(r" {2,}", " ", cleaned).strip()
5571

5672

57-
class EmojiStrippingFilter(logging.Filter):
58-
"""Logging filter that strips emoji/symbols from ``record.msg`` and ``record.args``.
73+
class LogSanitizingFilter(logging.Filter):
74+
"""Logging filter that sanitizes ``record.msg`` and ``record.args``.
5975
60-
Attach this to the root logger's handlers so every log record is sanitised
61-
before it reaches a ``StreamHandler`` (console / pipe). File-based handlers
76+
Attach this to the root logger's handlers so every log record is cleaned
77+
before it reaches a ``StreamHandler`` (console / pipe): emoji/symbols are
78+
removed and CR/LF + control characters are neutralised. File-based handlers
6279
with ``propagate=False`` (e.g. the Windows supervisor's own log) are not
63-
affected.
80+
affected. The exception traceback added by ``logger.exception`` lives in
81+
``record.exc_info`` and is rendered by the formatter after this filter runs,
82+
so it is never altered here — the full error always reaches the log.
6483
"""
6584

6685
def filter(self, record):
6786
if isinstance(record.msg, str):
68-
record.msg = _strip_emoji(record.msg)
87+
record.msg = _sanitize_log_text(record.msg)
6988
if record.args:
7089
if isinstance(record.args, dict):
7190
record.args = {
72-
k: _strip_emoji(v) if isinstance(v, str) else v
91+
k: _sanitize_log_text(v) if isinstance(v, str) else v
7392
for k, v in record.args.items()
7493
}
7594
elif isinstance(record.args, (list, tuple)):
7695
record.args = tuple(
77-
_strip_emoji(a) if isinstance(a, str) else a
96+
_sanitize_log_text(a) if isinstance(a, str) else a
7897
for a in record.args
7998
)
8099
return True
@@ -83,10 +102,11 @@ def filter(self, record):
83102
def configure_logging(level: int = logging.INFO) -> None:
84103
"""Install the project-wide root logger format. Idempotent.
85104
86-
An ``EmojiStrippingFilter`` is attached to every handler on the root logger,
87-
making all console / pipe output safe on Windows regardless of code-page.
105+
A ``LogSanitizingFilter`` is attached to every handler on the root logger,
106+
making all console / pipe output safe on Windows regardless of code-page and
107+
neutralising log-injection attempts from untrusted message data.
88108
"""
89109
logging.basicConfig(level=level, format=LOG_FORMAT, datefmt=LOG_DATEFMT)
90110
for handler in logging.root.handlers:
91-
if not any(isinstance(f, EmojiStrippingFilter) for f in handler.filters):
92-
handler.addFilter(EmojiStrippingFilter())
111+
if not any(isinstance(f, LogSanitizingFilter) for f in handler.filters):
112+
handler.addFilter(LogSanitizingFilter())

database.py

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,13 @@ def save_task_status(task_id, task_type, status=TASK_STATUS_PENDING, parent_task
304304
END
305305
""", (task_id, parent_task_id, task_type, sub_type_identifier, status, progress, details_json, current_unix_time, status, current_unix_time, current_unix_time, current_unix_time))
306306
db.commit()
307-
except psycopg2.Error as e:
308-
logger.error(f"DB Error saving task status for {task_id}: {e}")
307+
except psycopg2.Error:
308+
logger.exception(f"DB Error saving task status for {task_id}")
309309
try:
310310
db.rollback()
311311
logger.info(f"DB transaction rolled back for task status update of {task_id}.")
312-
except psycopg2.Error as rb_e:
313-
logger.error(f"DB Error during rollback for task status {task_id}: {rb_e}")
312+
except psycopg2.Error:
313+
logger.exception(f"DB Error during rollback for task status {task_id}")
314314
finally:
315315
cur.close()
316316

@@ -394,8 +394,8 @@ def get_score_data_by_ids(item_ids_list):
394394
try:
395395
cur.execute(query, (tuple(item_ids_list),))
396396
rows = cur.fetchall()
397-
except Exception as e:
398-
logger.error(f"Error fetching score data by IDs: {e}")
397+
except Exception:
398+
logger.exception("Error fetching score data by IDs")
399399
rows = []
400400
finally:
401401
cur.close()
@@ -491,8 +491,8 @@ def load_map_projection(index_name, force_reload=False):
491491
MAP_PROJECTION_CACHE = {'index_name': index_name, 'id_map': id_map, 'projection': proj}
492492
logger.info(f"Map projection '{index_name}' with {len(id_map)} items loaded successfully into memory.")
493493
return id_map, proj
494-
except Exception as e:
495-
logger.error(f"Failed to load map projection: {e}", exc_info=True)
494+
except Exception:
495+
logger.exception("Failed to load map projection")
496496
return None, None
497497
finally:
498498
cur.close()
@@ -611,9 +611,9 @@ def _parse_year_from_date(year_value):
611611
""", (item_id, psycopg2.Binary(embedding_blob)))
612612

613613
conn.commit()
614-
except Exception as e:
614+
except Exception:
615615
conn.rollback()
616-
logger.error("Error saving track analysis and embedding for %s: %s", item_id, e)
616+
logger.exception("Error saving track analysis and embedding for %s", item_id)
617617
raise
618618
finally:
619619
cur.close()
@@ -633,9 +633,9 @@ def save_clap_embedding(item_id, clap_embedding_vector):
633633
ON CONFLICT (item_id) DO UPDATE SET embedding = EXCLUDED.embedding
634634
""", (item_id, psycopg2.Binary(embedding_blob)))
635635
conn.commit()
636-
except Exception as e:
636+
except Exception:
637637
conn.rollback()
638-
logger.error(f"Error saving CLAP embedding for {item_id}: {e}")
638+
logger.exception(f"Error saving CLAP embedding for {item_id}")
639639
raise
640640
finally:
641641
cur.close()
@@ -655,8 +655,8 @@ def get_clap_embedding(item_id):
655655
if row and row[0]:
656656
return np.frombuffer(row[0], dtype=np.float32)
657657
return None
658-
except Exception as e:
659-
logger.error(f"Error loading CLAP embedding for {item_id}: {e}")
658+
except Exception:
659+
logger.exception(f"Error loading CLAP embedding for {item_id}")
660660
return None
661661
finally:
662662
cur.close()
@@ -686,9 +686,9 @@ def save_lyrics_embedding(item_id, lyrics_embedding_vector, axis_vector=None):
686686
""", (item_id, psycopg2.Binary(embedding_blob),
687687
psycopg2.Binary(axis_blob) if axis_blob is not None else None))
688688
conn.commit()
689-
except Exception as e:
689+
except Exception:
690690
conn.rollback()
691-
logger.error(f"Error saving lyrics embedding for {item_id}: {e}")
691+
logger.exception(f"Error saving lyrics embedding for {item_id}")
692692
raise
693693
finally:
694694
cur.close()
@@ -1121,9 +1121,9 @@ def clean_up_previous_main_tasks():
11211121
logger.info(f"Archived {archived_count} previous main tasks and deleted {deleted_children_count} child tasks.")
11221122
else:
11231123
logger.info("No previous main tasks found to clean up.")
1124-
except Exception as e_main_clean:
1124+
except Exception:
11251125
db.rollback()
1126-
logger.error(f"Error during the main task cleanup process: {e_main_clean}")
1126+
logger.exception("Error during the main task cleanup process")
11271127
finally:
11281128
cur.close()
11291129

@@ -1192,9 +1192,9 @@ def save_alchemy_anchor(name, centroid):
11921192
row = cur.fetchone()
11931193
conn.commit()
11941194
return dict(row) if row else None
1195-
except Exception as e:
1195+
except Exception:
11961196
conn.rollback()
1197-
logger.error(f"Failed to save alchemy anchor '{name}': {e}")
1197+
logger.exception(f"Failed to save alchemy anchor '{name}'")
11981198
return None
11991199
finally:
12001200
cur.close()
@@ -1206,8 +1206,8 @@ def get_alchemy_anchors():
12061206
cur.execute("SELECT id, name, created_at FROM alchemy_anchors ORDER BY created_at DESC")
12071207
rows = cur.fetchall()
12081208
return [dict(row) for row in rows]
1209-
except Exception as e:
1210-
logger.error(f"Failed to load alchemy anchors: {e}")
1209+
except Exception:
1210+
logger.exception("Failed to load alchemy anchors")
12111211
return []
12121212
finally:
12131213
cur.close()
@@ -1219,9 +1219,9 @@ def delete_alchemy_anchor(anchor_id):
12191219
cur.execute("DELETE FROM alchemy_anchors WHERE id = %s", (anchor_id,))
12201220
conn.commit()
12211221
return cur.rowcount > 0
1222-
except Exception as e:
1222+
except Exception:
12231223
conn.rollback()
1224-
logger.error(f"Failed to delete alchemy anchor id={anchor_id}: {e}")
1224+
logger.exception(f"Failed to delete alchemy anchor id={anchor_id}")
12251225
return False
12261226
finally:
12271227
cur.close()
@@ -1241,8 +1241,8 @@ def get_alchemy_anchor_by_id(anchor_id):
12411241
except Exception:
12421242
anchor['centroid'] = None
12431243
return anchor
1244-
except Exception as e:
1245-
logger.error(f"Failed to fetch alchemy anchor id={anchor_id}: {e}")
1244+
except Exception:
1245+
logger.exception(f"Failed to fetch alchemy anchor id={anchor_id}")
12461246
return None
12471247
finally:
12481248
cur.close()
@@ -1262,9 +1262,9 @@ def update_alchemy_anchor_name(anchor_id, name):
12621262
if not row:
12631263
return None
12641264
return dict(row)
1265-
except Exception as e:
1265+
except Exception:
12661266
conn.rollback()
1267-
logger.error(f"Failed to rename alchemy anchor id={anchor_id}: {e}")
1267+
logger.exception(f"Failed to rename alchemy anchor id={anchor_id}")
12681268
return None
12691269
finally:
12701270
cur.close()
@@ -1379,9 +1379,9 @@ def save_map_projection(index_name, id_map, projection_array):
13791379
logger.info(f"Saved map projection '{index_name}' to DB: {len(blob)} bytes, ids={id_count}")
13801380
except Exception:
13811381
logger.debug("Saved map projection but failed to compute size/id_count for log.")
1382-
except Exception as e:
1382+
except Exception:
13831383
conn.rollback()
1384-
logger.error(f"Failed to save map projection: {e}")
1384+
logger.exception("Failed to save map projection")
13851385
raise
13861386

13871387
def load_artist_projection(index_name='artist_map', force_reload=False):
@@ -1413,8 +1413,8 @@ def load_artist_projection(index_name='artist_map', force_reload=False):
14131413
ARTIST_PROJECTION_CACHE = {'index_name': index_name, 'component_map': component_map, 'projection': proj}
14141414
logger.info(f"Artist projection '{index_name}' with {len(component_map)} components loaded successfully into memory.")
14151415
return component_map, proj
1416-
except Exception as e:
1417-
logger.error(f"Failed to load artist projection: {e}", exc_info=True)
1416+
except Exception:
1417+
logger.exception("Failed to load artist projection")
14181418
return None, None
14191419
finally:
14201420
cur.close()
@@ -1432,9 +1432,9 @@ def save_artist_projection(index_name, component_map, projections):
14321432
cur.execute("INSERT INTO artist_component_projection (index_name, projection_data, artist_component_map_json) VALUES (%s, %s, %s) ON CONFLICT (index_name) DO UPDATE SET projection_data = EXCLUDED.projection_data, artist_component_map_json = EXCLUDED.artist_component_map_json, created_at = CURRENT_TIMESTAMP", (index_name, proj_blob, component_map_json))
14331433
conn.commit()
14341434
logger.info(f"Saved artist projection '{index_name}' with {len(component_map)} components to database.")
1435-
except Exception as e:
1435+
except Exception:
14361436
conn.rollback()
1437-
logger.error(f"Failed to save artist projection: {e}", exc_info=True)
1437+
logger.exception("Failed to save artist projection")
14381438
finally:
14391439
cur.close()
14401440

@@ -1454,8 +1454,8 @@ def update_playlist_table(playlists): # Removed db_path
14541454
for item_id, title, author in cluster:
14551455
cur.execute("INSERT INTO playlist (playlist_name, item_id, title, author) VALUES (%s, %s, %s, %s) ON CONFLICT (playlist_name, item_id) DO NOTHING", (name, item_id, title, author))
14561456
conn.commit()
1457-
except Exception as e:
1457+
except Exception:
14581458
conn.rollback()
1459-
logger.error("Error updating playlist table: %s", e)
1459+
logger.exception("Error updating playlist table")
14601460
finally:
14611461
cur.close()

0 commit comments

Comments
 (0)