From 7259708f280baaf70915917a806fcc821d3f22aa Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 14:10:28 -0400 Subject: [PATCH] Session length, trash retention and attachment size have bounds Each accepted any integer. A session length of 0 expired every session at once, the admin's own included; an attachment limit above the 64 MB body ceiling allowed files no request could carry, and a negative one refused every upload. - session_ttl_days 1..3650, trash_retention_days 0..3650 (0 = keep), and max_attachment_mb 1..MAX_BODY_MB-1, leaving room for the multipart envelope. - MAX_BODY_MB is the one number app.py's MAX_CONTENT_LENGTH and that maximum both read. - A value stored before its bounds existed reads as the nearest bound. Fixes #5384. Co-Authored-By: Claude Opus 5.5 --- src/inkwell/app.py | 6 +++--- src/inkwell/settings.py | 21 ++++++++++++++++++++- tests/test_settings.py | 25 ++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/inkwell/app.py b/src/inkwell/app.py index c85b37b..9c1c412 100644 --- a/src/inkwell/app.py +++ b/src/inkwell/app.py @@ -24,7 +24,7 @@ from .labels import bp as labels_bp from .notes import bp as notes_bp from .proxy import is_https from .retention import run_sweeper -from .settings import get_public_config, get_setting, load_or_create_secret_key, refresh_live +from .settings import MAX_BODY_MB, get_public_config, get_setting, load_or_create_secret_key, refresh_live from .settings_api import bp as settings_bp from .shares_api import bp as shares_bp from .sync import bp as sync_bp, protocol_advertisement @@ -91,8 +91,8 @@ def create_app() -> Quart: app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=30) # Hard request-body ceiling (any-file attachments, import zips, sync push). The # per-file attachment limit is the DB-backed `max_attachment_mb` setting, enforced - # in the upload handler; this must stay >= the largest value that allows. - app.config["MAX_CONTENT_LENGTH"] = 64 * 1024 * 1024 + # in the upload handler and bounded under this by its own maximum. + app.config["MAX_CONTENT_LENGTH"] = MAX_BODY_MB * 1024 * 1024 app.register_blueprint(auth_bp) app.register_blueprint(notes_bp) diff --git a/src/inkwell/settings.py b/src/inkwell/settings.py index 8deddd2..ccee00a 100644 --- a/src/inkwell/settings.py +++ b/src/inkwell/settings.py @@ -11,6 +11,11 @@ from .models.settings import Setting SettingType = Literal["string", "bool", "int"] +# The request-body ceiling, in MB (app.py's MAX_CONTENT_LENGTH). One upload's file +# has to fit inside it with its multipart envelope, which is why the largest +# attachment an admin may allow is a megabyte under it. +MAX_BODY_MB = 64 + @dataclass(frozen=True) class SettingDef: @@ -67,6 +72,9 @@ REGISTRY: list[SettingDef] = [ "Session length (days)", "How long a signed-in session stays valid before another login is required.", "Access", + # 0 or less would expire every session at once, the admin's own included. + minimum=1, + maximum=3650, ), SettingDef( "trash_retention_days", @@ -76,6 +84,8 @@ REGISTRY: list[SettingDef] = [ "How long a note stays in Trash before it's permanently deleted, freeing its " "attachments from disk. Set to 0 to keep trashed notes until they're deleted by hand.", "Notes", + minimum=0, + maximum=3650, ), SettingDef( "max_attachment_mb", @@ -84,6 +94,8 @@ REGISTRY: list[SettingDef] = [ "Max attachment size (MB)", "Largest single file that can be attached to a note. Capped by the server body limit.", "Attachments", + minimum=1, + maximum=MAX_BODY_MB - 1, ), SettingDef( "storage_quota_gb", @@ -254,9 +266,16 @@ def _coerce(defn: SettingDef, raw: Any) -> Any: return coerce_bool(raw) if defn.type == "int": try: - return int(raw) + n = int(raw) except (ValueError, TypeError): return defn.default + # A value saved before its bounds existed is still in the row; read it as the + # nearest bound rather than let it act (#5384). + if defn.minimum is not None: + n = max(n, defn.minimum) + if defn.maximum is not None: + n = min(n, defn.maximum) + return n return str(raw) diff --git a/tests/test_settings.py b/tests/test_settings.py index 4bf3cf3..9f2ac60 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -1,4 +1,4 @@ -from inkwell.settings import REGISTRY, validate_updates +from inkwell.settings import MAX_BODY_MB, REGISTRY, validate_updates def test_registry_has_expected_keys(): @@ -19,6 +19,29 @@ def test_validate_coerces_bool_and_int(): assert clean["session_ttl_days"] == 45 +def test_each_integer_with_a_dangerous_range_is_bounded(): + """Session length 0 expired every session at once; an attachment limit above the + body ceiling allowed files no request could carry; a negative one refused all (#5384).""" + for key, bad, good in ( + ("session_ttl_days", 0, 1), + ("trash_retention_days", -1, 0), + ("max_attachment_mb", 0, 1), + ("max_attachment_mb", MAX_BODY_MB, MAX_BODY_MB - 1), + ): + clean, error = validate_updates({key: str(bad)}) + assert error is not None and clean == {}, f"{key}={bad} refused" + clean, error = validate_updates({key: str(good)}) + assert error is None and clean == {key: good}, f"{key}={good} kept" + + +def test_a_stored_value_from_before_the_bounds_reads_as_the_nearest_bound(): + from inkwell.settings import _BY_KEY, _coerce + + assert _coerce(_BY_KEY["session_ttl_days"], 0) == 1 + assert _coerce(_BY_KEY["max_attachment_mb"], 500) == MAX_BODY_MB - 1 + assert _coerce(_BY_KEY["trash_retention_days"], 30) == 30 + + def test_validate_rejects_bad_int(): clean, error = validate_updates({"session_ttl_days": "not-a-number"}) assert error is not None