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 <noreply@anthropic.com>
This commit is contained in:
+3
-3
@@ -24,7 +24,7 @@ from .labels import bp as labels_bp
|
|||||||
from .notes import bp as notes_bp
|
from .notes import bp as notes_bp
|
||||||
from .proxy import is_https
|
from .proxy import is_https
|
||||||
from .retention import run_sweeper
|
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 .settings_api import bp as settings_bp
|
||||||
from .shares_api import bp as shares_bp
|
from .shares_api import bp as shares_bp
|
||||||
from .sync import bp as sync_bp, protocol_advertisement
|
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)
|
app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=30)
|
||||||
# Hard request-body ceiling (any-file attachments, import zips, sync push). The
|
# 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
|
# 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.
|
# in the upload handler and bounded under this by its own maximum.
|
||||||
app.config["MAX_CONTENT_LENGTH"] = 64 * 1024 * 1024
|
app.config["MAX_CONTENT_LENGTH"] = MAX_BODY_MB * 1024 * 1024
|
||||||
|
|
||||||
app.register_blueprint(auth_bp)
|
app.register_blueprint(auth_bp)
|
||||||
app.register_blueprint(notes_bp)
|
app.register_blueprint(notes_bp)
|
||||||
|
|||||||
+20
-1
@@ -11,6 +11,11 @@ from .models.settings import Setting
|
|||||||
|
|
||||||
SettingType = Literal["string", "bool", "int"]
|
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)
|
@dataclass(frozen=True)
|
||||||
class SettingDef:
|
class SettingDef:
|
||||||
@@ -67,6 +72,9 @@ REGISTRY: list[SettingDef] = [
|
|||||||
"Session length (days)",
|
"Session length (days)",
|
||||||
"How long a signed-in session stays valid before another login is required.",
|
"How long a signed-in session stays valid before another login is required.",
|
||||||
"Access",
|
"Access",
|
||||||
|
# 0 or less would expire every session at once, the admin's own included.
|
||||||
|
minimum=1,
|
||||||
|
maximum=3650,
|
||||||
),
|
),
|
||||||
SettingDef(
|
SettingDef(
|
||||||
"trash_retention_days",
|
"trash_retention_days",
|
||||||
@@ -76,6 +84,8 @@ REGISTRY: list[SettingDef] = [
|
|||||||
"How long a note stays in Trash before it's permanently deleted, freeing its "
|
"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.",
|
"attachments from disk. Set to 0 to keep trashed notes until they're deleted by hand.",
|
||||||
"Notes",
|
"Notes",
|
||||||
|
minimum=0,
|
||||||
|
maximum=3650,
|
||||||
),
|
),
|
||||||
SettingDef(
|
SettingDef(
|
||||||
"max_attachment_mb",
|
"max_attachment_mb",
|
||||||
@@ -84,6 +94,8 @@ REGISTRY: list[SettingDef] = [
|
|||||||
"Max attachment size (MB)",
|
"Max attachment size (MB)",
|
||||||
"Largest single file that can be attached to a note. Capped by the server body limit.",
|
"Largest single file that can be attached to a note. Capped by the server body limit.",
|
||||||
"Attachments",
|
"Attachments",
|
||||||
|
minimum=1,
|
||||||
|
maximum=MAX_BODY_MB - 1,
|
||||||
),
|
),
|
||||||
SettingDef(
|
SettingDef(
|
||||||
"storage_quota_gb",
|
"storage_quota_gb",
|
||||||
@@ -254,9 +266,16 @@ def _coerce(defn: SettingDef, raw: Any) -> Any:
|
|||||||
return coerce_bool(raw)
|
return coerce_bool(raw)
|
||||||
if defn.type == "int":
|
if defn.type == "int":
|
||||||
try:
|
try:
|
||||||
return int(raw)
|
n = int(raw)
|
||||||
except (ValueError, TypeError):
|
except (ValueError, TypeError):
|
||||||
return defn.default
|
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)
|
return str(raw)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+24
-1
@@ -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():
|
def test_registry_has_expected_keys():
|
||||||
@@ -19,6 +19,29 @@ def test_validate_coerces_bool_and_int():
|
|||||||
assert clean["session_ttl_days"] == 45
|
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():
|
def test_validate_rejects_bad_int():
|
||||||
clean, error = validate_updates({"session_ttl_days": "not-a-number"})
|
clean, error = validate_updates({"session_ttl_days": "not-a-number"})
|
||||||
assert error is not None
|
assert error is not None
|
||||||
|
|||||||
Reference in New Issue
Block a user