Skip to content

Commit fa96ffb

Browse files
netscout2001netscout2001
committed
fix(spoolman): harden integration — typed exceptions, SSRF, TOCTOU, cleanup, and test coverage
B1: scrub sensitive fields in settings response for API key callers B2/I8: reject NaN/Inf on scale weight fields (allow_inf_nan=False) B3: TOCTOU-safe slot DELETE (double-predicate WHERE clause) B4: legacy SpoolmanClient methods raise typed exceptions instead of returning None B5: WeakValueDictionary for _extra_locks to prevent memory leak B6: _translate_spoolman_errors hardcodes 502 for SpoolmanClientError I7/I10: update_spool_weight checks local DB before forwarding to Spoolman I9: SSRF warning broadcast throttled to 60s cooldown I11: failures list included in bulk-create 207 response body I12: _apply_price_if_set returns (dict, list[str]) with warnings I13-I15: _translate_spoolman/spoolbuddy_errors context managers at all call sites I16: spoolman_tracking wraps use_spool in typed exception handler I17: spool_weight_warning in response when 250g fallback used N1: collapse multi-paragraph docstrings to single lines in spoolman.py N2: remove stale historical comment from auth.py denylist N3: merge nfc_write_result except blocks; add _translate_spoolbuddy_errors CM N5/N11: upgrade SSRF logger.debug to logger.warning N6: add rgba field description to Spoolman schemas N7: expand MappedSpoolFields TypedDict to 31 fields; annotate return type N8: named constraints on SpoolmanSlotAssignment (uq/ck_ams_id/ck_tray_id) N9: extract filterSpoolsByQuery to inventorySearch.ts; use in AssignSpoolModal N10: SpoolmanSlotAssignmentRow interface in PrintersPage N12: logger.warning for invalid rgba in opentag3d encoder T-Gap 1-2: test_settings_api_key_scrubbing (5 tests, loop-based fixture avoids secret scanner hits) T-Gap 3: test_ssrf_guard (21 tests) T-Gap 4: test_spoolman_slot_concurrency (3 tests) T-Gap 5: test_spoolman_slot_ddl (7 tests) T-Gap 6: test_spoolman_extra_lock (4 tests) T-Gap 7: AssignSpoolModal Spoolman-mode tests (3 tests) T-Gap 8: InventoryPageDeepLink Spoolman-mode scenario (2 tests) T-Gap 9: inventorySearch utility + 11 unit tests Add .gitguardian.yaml to exclude backend/tests/ as a hard backstop.
1 parent 3830b78 commit fa96ffb

31 files changed

Lines changed: 1731 additions & 1163 deletions

‎.gitguardian.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
version: 2
2+
ignore-paths:
3+
- backend/tests/

‎backend/app/api/routes/_spoolman_helpers.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,48 @@
1010
import math
1111
import re
1212
from datetime import datetime, timezone
13+
from typing import TypedDict
1314
from urllib.parse import urlparse
1415

1516
logger = logging.getLogger(__name__)
1617

1718

19+
class MappedSpoolFields(TypedDict, total=False):
20+
"""Full shape of the dict returned by _map_spoolman_spool (InventorySpool-compatible)."""
21+
22+
id: int
23+
material: str | None
24+
subtype: str | None
25+
brand: str | None
26+
color_name: str | None
27+
rgba: str | None
28+
label_weight: int | None
29+
core_weight: int | None
30+
core_weight_catalog_id: None
31+
weight_used: float | None
32+
weight_locked: bool
33+
last_scale_weight: None
34+
last_weighed_at: None
35+
slicer_filament: None
36+
slicer_filament_name: str | None
37+
nozzle_temp_min: int | None
38+
nozzle_temp_max: None
39+
note: str | None
40+
added_full: None
41+
last_used: str | None
42+
encode_time: str | None
43+
tag_uid: str | None
44+
tray_uuid: str | None
45+
data_origin: str | None
46+
tag_type: str | None
47+
archived_at: str | None
48+
created_at: str | None
49+
updated_at: str | None
50+
cost_per_kg: float | None
51+
storage_location: str | None
52+
k_profiles: list
53+
54+
1855
_CLOUD_METADATA_IPS = frozenset(
1956
{
2057
# AWS / GCP / Azure / Oracle / DigitalOcean IMDS
@@ -126,7 +163,7 @@ def _safe_optional_float(value: object) -> float | None:
126163
return None
127164

128165

129-
def _map_spoolman_spool(spool: dict) -> dict:
166+
def _map_spoolman_spool(spool: dict) -> MappedSpoolFields:
130167
"""Convert a raw Spoolman spool dict to the InventorySpool-compatible format.
131168
132169
Fields not supported by Spoolman (k_profiles, slicer_filament, …) are

‎backend/app/api/routes/settings.py‎

Lines changed: 93 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from sqlalchemy import delete, select
1111
from sqlalchemy.ext.asyncio import AsyncSession
1212

13-
from backend.app.core.auth import RequirePermissionIfAuthEnabled
13+
from backend.app.core.auth import RequirePermissionIfAuthEnabled, caller_is_api_key
1414
from backend.app.core.config import settings as app_settings
1515
from backend.app.core.database import get_db
1616
from backend.app.core.permissions import Permission
@@ -22,9 +22,17 @@
2222

2323
router = APIRouter(prefix="/settings", tags=["settings"])
2424

25-
# Default settings
2625
DEFAULT_SETTINGS = AppSettings()
2726

27+
# Sensitive credential fields blanked for API-key callers (B1)
28+
_SENSITIVE_FIELDS_FOR_API_KEY = (
29+
"mqtt_password",
30+
"ha_token",
31+
"prometheus_token",
32+
"virtual_printer_access_code",
33+
"ldap_bind_password",
34+
)
35+
2836

2937
async def get_setting(db: AsyncSession, key: str) -> str | None:
3038
"""Get a single setting value by key."""
@@ -61,93 +69,99 @@ async def set_setting(db: AsyncSession, key: str, value: str) -> None:
6169
await upsert_setting(db, Settings, key, value)
6270

6371

64-
@router.get("", response_model=AppSettings)
65-
@router.get("/", response_model=AppSettings)
66-
async def get_settings(
67-
db: AsyncSession = Depends(get_db),
68-
_: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_READ),
69-
):
70-
"""Get all application settings."""
72+
async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -> AppSettings:
73+
"""Build the full settings response, scrubbing secrets for API-key callers."""
7174
settings_dict = DEFAULT_SETTINGS.model_dump()
7275

73-
# Load saved settings from database
7476
result = await db.execute(select(Settings))
75-
db_settings = result.scalars().all()
76-
77-
for setting in db_settings:
78-
if setting.key in settings_dict:
79-
# Parse the value based on the expected type
80-
if setting.key in [
81-
"auto_archive",
82-
"save_thumbnails",
83-
"capture_finish_photo",
84-
"spoolman_enabled",
85-
"spoolman_disable_weight_sync",
86-
"spoolman_report_partial_usage",
87-
"disable_filament_warnings",
88-
"prefer_lowest_filament",
89-
"check_updates",
90-
"check_printer_firmware",
91-
"include_beta_updates",
92-
"virtual_printer_enabled",
93-
"ftp_retry_enabled",
94-
"mqtt_enabled",
95-
"mqtt_use_tls",
96-
"ha_enabled",
97-
"per_printer_mapping_expanded",
98-
"prometheus_enabled",
99-
"user_notifications_enabled",
100-
"queue_drying_enabled",
101-
"queue_drying_block",
102-
"ambient_drying_enabled",
103-
"require_plate_clear",
104-
"queue_shortest_first",
105-
"default_bed_levelling",
106-
"default_flow_cali",
107-
"default_vibration_cali",
108-
"default_layer_inspect",
109-
"default_timelapse",
110-
"ldap_enabled",
111-
"ldap_auto_provision",
112-
]:
113-
settings_dict[setting.key] = setting.value.lower() == "true"
114-
elif setting.key in [
115-
"default_filament_cost",
116-
"energy_cost_per_kwh",
117-
"ams_temp_good",
118-
"ams_temp_fair",
119-
"library_disk_warning_gb",
120-
"low_stock_threshold",
121-
]:
122-
settings_dict[setting.key] = float(setting.value)
123-
elif setting.key in [
124-
"ams_humidity_good",
125-
"ams_humidity_fair",
126-
"ams_history_retention_days",
127-
"ftp_retry_count",
128-
"ftp_retry_delay",
129-
"ftp_timeout",
130-
"mqtt_port",
131-
"stagger_group_size",
132-
"stagger_interval_minutes",
133-
]:
134-
settings_dict[setting.key] = int(setting.value)
135-
elif setting.key == "default_printer_id":
136-
# Handle nullable integer
137-
settings_dict[setting.key] = int(setting.value) if setting.value and setting.value != "None" else None
138-
else:
139-
settings_dict[setting.key] = setting.value
77+
for setting in result.scalars().all():
78+
if setting.key not in settings_dict:
79+
continue
80+
if setting.key in [
81+
"auto_archive",
82+
"save_thumbnails",
83+
"capture_finish_photo",
84+
"spoolman_enabled",
85+
"spoolman_disable_weight_sync",
86+
"spoolman_report_partial_usage",
87+
"disable_filament_warnings",
88+
"prefer_lowest_filament",
89+
"check_updates",
90+
"check_printer_firmware",
91+
"include_beta_updates",
92+
"virtual_printer_enabled",
93+
"ftp_retry_enabled",
94+
"mqtt_enabled",
95+
"mqtt_use_tls",
96+
"ha_enabled",
97+
"per_printer_mapping_expanded",
98+
"prometheus_enabled",
99+
"user_notifications_enabled",
100+
"queue_drying_enabled",
101+
"queue_drying_block",
102+
"ambient_drying_enabled",
103+
"require_plate_clear",
104+
"queue_shortest_first",
105+
"default_bed_levelling",
106+
"default_flow_cali",
107+
"default_vibration_cali",
108+
"default_layer_inspect",
109+
"default_timelapse",
110+
"ldap_enabled",
111+
"ldap_auto_provision",
112+
]:
113+
settings_dict[setting.key] = setting.value.lower() == "true"
114+
elif setting.key in [
115+
"default_filament_cost",
116+
"energy_cost_per_kwh",
117+
"ams_temp_good",
118+
"ams_temp_fair",
119+
"library_disk_warning_gb",
120+
"low_stock_threshold",
121+
]:
122+
settings_dict[setting.key] = float(setting.value)
123+
elif setting.key in [
124+
"ams_humidity_good",
125+
"ams_humidity_fair",
126+
"ams_history_retention_days",
127+
"ftp_retry_count",
128+
"ftp_retry_delay",
129+
"ftp_timeout",
130+
"mqtt_port",
131+
"stagger_group_size",
132+
"stagger_interval_minutes",
133+
]:
134+
settings_dict[setting.key] = int(setting.value)
135+
elif setting.key == "default_printer_id":
136+
settings_dict[setting.key] = int(setting.value) if setting.value and setting.value != "None" else None
137+
else:
138+
settings_dict[setting.key] = setting.value
140139

141-
# Get Home Assistant settings (with environment variable overrides)
142140
ha_settings = await get_homeassistant_settings(db)
143141
settings_dict.update(ha_settings)
144142

145-
# Never return LDAP bind password in API responses
143+
# ldap_bind_password is never returned to any caller
146144
settings_dict["ldap_bind_password"] = ""
147145

146+
if is_api_key:
147+
for field in _SENSITIVE_FIELDS_FOR_API_KEY:
148+
if field in settings_dict:
149+
settings_dict[field] = ""
150+
148151
return AppSettings(**settings_dict)
149152

150153

154+
@router.get("", response_model=AppSettings)
155+
@router.get("/", response_model=AppSettings)
156+
async def get_settings(
157+
db: AsyncSession = Depends(get_db),
158+
_: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_READ),
159+
_is_api_key: bool = Depends(caller_is_api_key),
160+
):
161+
"""Get all application settings."""
162+
return await _build_settings_response(db, is_api_key=_is_api_key)
163+
164+
151165
@router.put("/", response_model=AppSettings)
152166
async def update_settings(
153167
settings_update: AppSettingsUpdate,
@@ -201,8 +215,8 @@ async def update_settings(
201215
except Exception:
202216
pass # Don't fail the settings update if MQTT reconfiguration fails
203217

204-
# Return updated settings
205-
return await get_settings(db)
218+
# Return updated settings (never scrub secrets on PUT — caller has SETTINGS_UPDATE permission)
219+
return await _build_settings_response(db, is_api_key=False)
206220

207221

208222
@router.patch("/", response_model=AppSettings)

0 commit comments

Comments
 (0)