public_git/bambuddy
1
0
Fork 0
mirror of https://github.com/maziggy/bambuddy.git synced 2026-10-18 21:47:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refuse null for number and on/off settings, and read bad rows as the default (issue #2518)

Browse Source 
A PUT /settings with null for a number setting stored the literal
    "None". From then on int()/float() raised inside the response builder,
    and every settings read returned 503 until the row was fixed by hand.

    Explicit null for any boolean or numeric setting is now refused with a
    422 that names the keys, and nothing in that request is saved. A numeric
    row that does not parse reads back as its default with a warning, so an
    install that already stored one recovers. The typed-key lists moved to
    module constants so the save check and the read path share them.
This commit is contained in:
maziggy
2026-09-30 16:17:14 +02:00
parent 86a0d0deb7 0cc7cf0f36
commit 9bb2a01f9e
3 changed files with 158 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/api/routes/settings.py
+112 -80
View File
@@ -167,6 +167,96 @@ async def set_setting(db: AsyncSession, key: str, value: str) -> None:
await upsert_setting(db, Settings, key, value)
# Settings stored as booleans / numbers. Storage is a VARCHAR column, so
# _build_settings_response() parses these back, and update_settings() refuses
# an explicit null for them: a null is stored as the literal "None", which
# reads back as False for a boolean and is not a number at all.
_BOOL_SETTING_KEYS = frozenset(
{
"auto_archive",
"save_thumbnails",
"capture_finish_photo",
"finish_photo_restore_plate",
"spoolman_enabled",
"spoolman_disable_weight_sync",
"spoolman_report_partial_usage",
"auto_add_unknown_rfid",
"disable_filament_warnings",
"prefer_lowest_filament",
"check_updates",
"check_printer_firmware",
"include_beta_updates",
"virtual_printer_enabled",
"ftp_retry_enabled",
"mqtt_enabled",
"mqtt_use_tls",
"ha_enabled",
"per_printer_mapping_expanded",
"prometheus_enabled",
"user_notifications_enabled",
"queue_drying_enabled",
"queue_drying_block",
"ambient_drying_enabled",
"print_drying_enabled",
"require_plate_clear",
"queue_shortest_first",
# default_bed_levelling / default_flow_cali / default_nozzle_offset_cali
# are tri-state strings (off/on/auto) — parsed via the raw-string else
# branch; the TriState validator coerces legacy "true"/"false" rows.
"default_vibration_cali",
"default_layer_inspect",
"default_timelapse",
"default_confirm_outcome",
"confirm_outcome_external_prints",
"confirm_default_good_on_plate_clear",
"billing_enabled",
"printer_kill_switch_enabled",
"ldap_enabled",
"ldap_auto_provision",
"local_login_enabled",
"preheat_enabled",
"queue_keep_bed_warm",
}
)
_FLOAT_SETTING_KEYS = frozenset(
{
"default_filament_cost",
"energy_cost_per_kwh",
"ams_temp_good",
"ams_temp_fair",
"library_disk_warning_gb",
"low_stock_threshold",
}
)
_INT_SETTING_KEYS = frozenset(
{
"ams_humidity_good",
"ams_humidity_fair",
"ams_history_retention_days",
"printer_sensor_history_retention_days",
"ftp_retry_count",
"ftp_retry_delay",
"ftp_timeout",
"mqtt_port",
"stagger_group_size",
"stagger_interval_minutes",
"forecast_global_lead_time_days",
"location_sensor_poll_interval",
"finance_budget_reset_day",
"session_max_hours",
"pipeline_max_copies",
"preheat_max_wait_seconds",
"preheat_soak_seconds",
"queue_keep_warm_bed_temp",
"queue_keep_warm_max_minutes",
"queue_max_concurrent_uploads",
"ambient_drying_sustained_minutes",
}
)
async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -> AppSettings:
"""Build the full settings response, scrubbing secrets for API-key callers."""
settings_dict = DEFAULT_SETTINGS.model_dump()
@@ -175,95 +265,27 @@ async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -
for setting in result.scalars().all():
if setting.key not in settings_dict:
continue
if setting.key in [
"auto_archive",
"save_thumbnails",
"capture_finish_photo",
"finish_photo_restore_plate",
"spoolman_enabled",
"spoolman_disable_weight_sync",
"spoolman_report_partial_usage",
"auto_add_unknown_rfid",
"disable_filament_warnings",
"prefer_lowest_filament",
"check_updates",
"check_printer_firmware",
"include_beta_updates",
"virtual_printer_enabled",
"ftp_retry_enabled",
"mqtt_enabled",
"mqtt_use_tls",
"ha_enabled",
"per_printer_mapping_expanded",
"prometheus_enabled",
"user_notifications_enabled",
"queue_drying_enabled",
"queue_drying_block",
"ambient_drying_enabled",
"print_drying_enabled",
"require_plate_clear",
"queue_shortest_first",
# default_bed_levelling / default_flow_cali / default_nozzle_offset_cali
# are tri-state strings (off/on/auto) — parsed via the raw-string else
# branch; the TriState validator coerces legacy "true"/"false" rows.
"default_vibration_cali",
"default_layer_inspect",
"default_timelapse",
"default_confirm_outcome",
"confirm_outcome_external_prints",
"confirm_default_good_on_plate_clear",
"billing_enabled",
"printer_kill_switch_enabled",
"ldap_enabled",
"ldap_auto_provision",
"local_login_enabled",
"preheat_enabled",
"queue_keep_bed_warm",
]:
if setting.key in _BOOL_SETTING_KEYS:
settings_dict[setting.key] = setting.value.lower() == "true"
elif setting.key in [
"default_filament_cost",
"energy_cost_per_kwh",
"ams_temp_good",
"ams_temp_fair",
"library_disk_warning_gb",
"low_stock_threshold",
]:
settings_dict[setting.key] = float(setting.value)
elif setting.key in _FLOAT_SETTING_KEYS or setting.key in _INT_SETTING_KEYS:
# A value that does not parse (the literal "None" from an old
# null save, or a hand-edited row) keeps the default instead of
# taking the whole settings response down with it.
parse = int if setting.key in _INT_SETTING_KEYS else float
try:
settings_dict[setting.key] = parse(setting.value)
except (TypeError, ValueError):
logger.warning("Setting %s has an unparseable value; using the default", setting.key)
elif setting.key in [
# Nullable floats. Settings storage stringifies None to the literal
# "None", so these cannot go in the list above -- float("None")
# raises and would take the whole settings response with it (#2905).
# "None", which must read back as null here -- not as the default
# the _FLOAT_SETTING_KEYS branch above falls back to (#2905).
"ams_temp_alarm",
]:
try:
settings_dict[setting.key] = float(setting.value)
except (TypeError, ValueError):
settings_dict[setting.key] = None
elif setting.key in [
"ams_humidity_good",
"ams_humidity_fair",
"ams_history_retention_days",
"printer_sensor_history_retention_days",
"ftp_retry_count",
"ftp_retry_delay",
"ftp_timeout",
"mqtt_port",
"stagger_group_size",
"stagger_interval_minutes",
"forecast_global_lead_time_days",
"location_sensor_poll_interval",
"finance_budget_reset_day",
"session_max_hours",
"pipeline_max_copies",
"preheat_max_wait_seconds",
"preheat_soak_seconds",
"queue_keep_warm_bed_temp",
"queue_keep_warm_max_minutes",
"queue_max_concurrent_uploads",
"ambient_drying_sustained_minutes",
]:
settings_dict[setting.key] = int(setting.value)
elif setting.key == "default_printer_id":
settings_dict[setting.key] = int(setting.value) if setting.value and setting.value != "None" else None
elif setting.key == "open_in_slicer":
@@ -308,6 +330,16 @@ async def update_settings(
"""Update application settings."""
update_data = settings_update.model_dump(exclude_unset=True)
# An explicit null for a boolean or numeric setting has no meaning -- these
# are not clearable -- and would be stored as the literal "None".
null_keys = sorted(
key
for key, value in update_data.items()
if value is None and key in (_BOOL_SETTING_KEYS | _FLOAT_SETTING_KEYS | _INT_SETTING_KEYS)
)
if null_keys:
raise HTTPException(status_code=422, detail=f"These settings cannot be null: {', '.join(null_keys)}")
# Safety refusals on disabling local login (#1589). Two failure modes
# would otherwise lock everyone out of the install:
# 1. No enabled OIDC provider exists — nobody could authenticate.
backend/tests/integration/test_settings_api.py
+44
View File
@@ -61,6 +61,50 @@ class TestSettingsAPI:
assert response.status_code == 200
assert response.json()["ams_temp_alarm"] is None
@pytest.mark.asyncio
@pytest.mark.integration
@pytest.mark.parametrize(
("key", "default"),
[
("ambient_drying_sustained_minutes", 0),
("queue_keep_warm_bed_temp", 90),
("default_filament_cost", 25.0),
],
)
async def test_unparseable_number_setting_reads_back_as_the_default(
self, async_client: AsyncClient, db_session, key, default
):
"""A numeric row holding "None" (an old null save) or any other
unparseable value must fall back to the default, not make int()/float()
raise inside the response builder and take every setting with it."""
from backend.app.models.settings import Settings
db_session.add(Settings(key=key, value="None"))
await db_session.commit()
response = await async_client.get("/api/v1/settings/")
assert response.status_code == 200
assert response.json()[key] == default
@pytest.mark.asyncio
@pytest.mark.integration
@pytest.mark.parametrize("key", ["ambient_drying_sustained_minutes", "default_filament_cost", "auto_archive"])
async def test_null_for_a_typed_setting_is_refused_and_not_stored(self, async_client: AsyncClient, key):
"""A null for a boolean or numeric setting would be stored as the literal
"None". The whole request is refused, so a valid field sent alongside it
is not half-applied either."""
before = (await async_client.get("/api/v1/settings/")).json()
assert before["currency"] != "EUR"
response = await async_client.put("/api/v1/settings/", json={key: None, "currency": "EUR"})
assert response.status_code == 422
assert key in response.json()["detail"]
after = (await async_client.get("/api/v1/settings/")).json()
assert after[key] == before[key]
assert after["currency"] == before["currency"]
@pytest.mark.asyncio
@pytest.mark.integration
async def test_a_set_temp_alarm_reads_back_as_a_float(self, async_client: AsyncClient, db_session):