From c2c49182e6d4f1f1ef3ed3ba5e8699912f9a27bd Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 19 Sep 2026 01:02:05 +0200 Subject: feat(node): per-file upload ceiling is an operator setting, default 8 GB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was a 4 GB constant in webrtc_server.py, the same on a small board and on a machine holding a library. Now max_upload_gb in node.toml, on the Node page and via `meshbay-node transfers max-size`, read from the transport context per chunk so a change reaches an upload already running. MESHBAY_DESIGN.md §6.4; §15.3 records a defect found beside it. Co-Authored-By: Claude Opus 5 --- packages/meshbay-node/src/meshbay_node/config.py | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'packages/meshbay-node/src/meshbay_node/config.py') diff --git a/packages/meshbay-node/src/meshbay_node/config.py b/packages/meshbay-node/src/meshbay_node/config.py index 086d800..bf62639 100644 --- a/packages/meshbay-node/src/meshbay_node/config.py +++ b/packages/meshbay-node/src/meshbay_node/config.py @@ -67,6 +67,13 @@ max_concurrent_streams = 8 max_concurrent_downloads = 8 max_concurrent_uploads = 8 +# The largest single file a member may upload to this node, in GB. It is this +# machine's disk that fills, so the ceiling is the operator's to set: lower it +# on a small disk, raise it for a library of films. Fractions are allowed +# (0.5 = 512 MB). A file past it is refused at the chunk that crosses it and +# the partial file is deleted. +max_upload_gb = 8 + # HEVC sources have no browser decoder on most platforms, so streaming one is # transcoded to H264 rather than the usual free copy — real CPU per viewer. # Set to false only if every viewer's client is known to decode HEVC itself. @@ -181,6 +188,11 @@ class NodeConfig: # answer "server busy". See meshbay_node/transfers.py. max_concurrent_downloads: int = 8 max_concurrent_uploads: int = 8 + # The per-file upload ceiling, in GB — the one limit here that bounds a + # member's writes to the operator's disk rather than this node's own + # concurrency. There is still no aggregate quota (MESHBAY_DESIGN.md + # §15.3), so this is what stands between a writable root and a full disk. + max_upload_gb: float = 8.0 # HEVC (and any future codec in media_probe.py's # BROWSER_INCOMPATIBLE_VIDEO_CODECS) has no decoder in most browsers, so # streaming it needs a real re-encode to H264 rather than the usual free @@ -301,6 +313,28 @@ def _positive(value: object, default: int, name: str) -> int: return n +def _positive_float(value: object, default: float, name: str) -> float: + """A size that must be greater than zero, or the default with a word about it. + + Separate from `_positive` because this one is a quantity, not a count: half + a gigabyte is a legitimate ceiling on a small disk, and rounding it to zero + would refuse every upload with nothing in the log to say why. + """ + if isinstance(value, bool) or not isinstance(value, (int, float, str)): + log.warning("%s = %r is not a size — using %g", name, value, default) + return default + try: + n = float(value) + except (TypeError, ValueError): + log.warning("%s = %r is not a number — using %g", name, value, default) + return default + if n <= 0: + log.warning("%s = %g would refuse every upload — using %g", + name, n, default) + return default + return n + + def _read_roots(group: dict) -> list[RootSpec]: """ A group's roots, from `[[groups.roots]]` or from the legacy `shared_dir`. @@ -368,6 +402,9 @@ def load_config(path: Path = DEFAULT_CONFIG_PATH) -> Config: cfg.node.max_concurrent_uploads = _positive( nd.get("max_concurrent_uploads", cfg.node.max_concurrent_uploads), cfg.node.max_concurrent_uploads, "max_concurrent_uploads") + cfg.node.max_upload_gb = _positive_float( + nd.get("max_upload_gb", cfg.node.max_upload_gb), + cfg.node.max_upload_gb, "max_upload_gb") cfg.node.transcode_incompatible_video = bool( nd.get("transcode_incompatible_video", cfg.node.transcode_incompatible_video)) cfg.node.hardware_video_encode = bool( -- cgit v1.2.3 From 933daccbcfde7705413d3a10db87d910c650ed42 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 19 Sep 2026 08:54:52 +0200 Subject: fix(node): node.toml's transfer pools reach the transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon built the defaults dict for `Roster.node_settings` by hand and left out `max_concurrent_downloads` and `max_concurrent_uploads`. Absent from the dict, both resolved to None, were assigned back onto the config, and the transport skipped them — so node.toml was parsed, validated, and then replaced by `transfers.py`'s own 8. Invisible to anyone who left the value at 8, which is the value the template suggests. There were three copies of that dict and they all disagreed: node_status' was missing those two and `max_upload_gb` besides. One builder now, `config.node_settings_defaults`, and the resolver's key list is a class attribute the tests hold it to, along with the writer's. 1429 passed against a baseline of 1423; the two new behavioural tests fail with the builder reverted. Co-Authored-By: Claude Opus 5 --- docs/MESHBAY_DESIGN.md | 1 - packages/meshbay-node/src/meshbay_node/config.py | 29 ++++++++ packages/meshbay-node/src/meshbay_node/daemon.py | 15 +--- packages/meshbay-node/src/meshbay_node/ops.py | 67 ++++++++--------- packages/meshbay-node/src/meshbay_node/roster.py | 53 ++++++++++---- .../tests/test_node_settings_defaults.py | 85 ++++++++++++++++++++++ 6 files changed, 184 insertions(+), 66 deletions(-) create mode 100644 packages/meshbay-node/tests/test_node_settings_defaults.py (limited to 'packages/meshbay-node/src/meshbay_node/config.py') diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index d44de56..ce6a822 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -3104,7 +3104,6 @@ process runs it — `systemctl --user` on Linux, Task Scheduler on Windows. | **T3** for browser users | **Accepted permanently.** Removed for native clients, and that removal's value depends on reproducible builds | | **Hub identity pinning** (O13) | Nothing pins the hub's key. Bounded, because a substituted hub can neither read content nor ship code to a native client | | **Aggregate upload quota** | Per-file caps exist and the operator sets theirs (§6.4); a per-user or per-group total does not | -| **`max_concurrent_downloads` / `max_concurrent_uploads` are lost at startup** | The daemon builds the `defaults` dict it hands `roster.node_settings` without those two keys, so a node that has never had them set from the panel resolves both to `None`, assigns that back onto the config, and builds the transport with it — the pools then come up at `transfers.py`'s defaults and **what node.toml says about them is read and ignored**. The Node page and the CLI still work, because writing the setting is what creates the roster row the startup path then finds. Found while adding the per-file ceiling beside them, which is in that dict | | **A signed upload transcript** | Ownership is recorded by the node and verifiable by nobody else (§5.4). Making it provable is a transcript the uploader signs, stored with the entry — designed in outline, not built | | Forward secrecy in group chat | **Given up deliberately and on the record** (§4.5). If it becomes a requirement it belongs in 1:1 DM | | Metadata at the hub | Membership, and who posted in which group and when. A known leak, not a solved problem (§7.1) | diff --git a/packages/meshbay-node/src/meshbay_node/config.py b/packages/meshbay-node/src/meshbay_node/config.py index bf62639..2ae3c7c 100644 --- a/packages/meshbay-node/src/meshbay_node/config.py +++ b/packages/meshbay-node/src/meshbay_node/config.py @@ -366,6 +366,35 @@ def _read_roots(group: dict) -> list[RootSpec]: return specs +def node_settings_defaults(nd: NodeConfig | None = None) -> dict: + """ + The `node.toml` side of every setting the roster resolves. + + One function because there were three copies of this dict written by hand + and they disagreed. The daemon's left out both transfer pools, so on a node + whose operator had never touched the panel they resolved to None, were + written back onto the config, and the transport fell through to its own + defaults — `node.toml` parsed, validated, and then ignored. + + `test_node_settings_defaults.py` holds this against + `Roster.node_setting_keys()`, because a key missing here raises nothing + anywhere: it is a setting that stops working quietly. + """ + nd = nd or NodeConfig() + return { + "invite_ttl_hours": nd.invite_ttl_hours, + "pair_ttl_hours": nd.pair_ttl_hours, + "device_request_ttl_minutes": nd.device_request_ttl_minutes, + "max_concurrent_streams": nd.max_concurrent_streams, + "max_concurrent_downloads": nd.max_concurrent_downloads, + "max_concurrent_uploads": nd.max_concurrent_uploads, + "max_upload_gb": nd.max_upload_gb, + "transcode_incompatible_video": nd.transcode_incompatible_video, + "stun_servers": nd.stun_servers or list(DEFAULT_STUN_SERVERS), + "ice_interfaces": nd.ice_interfaces, + } + + def load_config(path: Path = DEFAULT_CONFIG_PATH) -> Config: """ Load config from TOML file. Supports both single [group] and diff --git a/packages/meshbay-node/src/meshbay_node/daemon.py b/packages/meshbay-node/src/meshbay_node/daemon.py index 1525205..478faae 100644 --- a/packages/meshbay-node/src/meshbay_node/daemon.py +++ b/packages/meshbay-node/src/meshbay_node/daemon.py @@ -293,19 +293,10 @@ class NodeDaemon: # Apply any roster overrides to node config (panel-edited values # take precedence over node.toml defaults). - from meshbay_node.config import DEFAULT_STUN_SERVERS + from meshbay_node.config import node_settings_defaults nd = self._config.node - defaults = { - "invite_ttl_hours": nd.invite_ttl_hours, - "pair_ttl_hours": nd.pair_ttl_hours, - "device_request_ttl_minutes": nd.device_request_ttl_minutes, - "max_concurrent_streams": nd.max_concurrent_streams, - "max_upload_gb": nd.max_upload_gb, - "transcode_incompatible_video": nd.transcode_incompatible_video, - "stun_servers": nd.stun_servers if nd.stun_servers else list(DEFAULT_STUN_SERVERS), - "ice_interfaces": nd.ice_interfaces, - } - effective = await self._roster.node_settings(defaults) + effective = await self._roster.node_settings( + node_settings_defaults(nd)) for k, v in effective.items(): setattr(nd, k, v) diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py index 9f0e6d1..b95ee2b 100644 --- a/packages/meshbay-node/src/meshbay_node/ops.py +++ b/packages/meshbay-node/src/meshbay_node/ops.py @@ -40,6 +40,7 @@ from meshbay_common.crypto import ( from meshbay_node.config import DEFAULT_CONFIG_PATH from meshbay_common.join import ROLE_MEMBER, ROLE_OPERATOR from meshbay_node.roots import RootError, RootSet, off_disk +from meshbay_node.roster import Roster log = logging.getLogger(__name__) @@ -623,17 +624,11 @@ async def list_groups(state: dict) -> dict: members = await roster.list_members() has_operator = any(m["role"] == "operator" and m["status"] == "active" for m in members) - from meshbay_node.config import DEFAULT_STUN_SERVERS - nd = config.node if config else None - defaults = { - "invite_ttl_hours": nd.invite_ttl_hours if nd else 168, - "pair_ttl_hours": nd.pair_ttl_hours if nd else 24, - "device_request_ttl_minutes": nd.device_request_ttl_minutes if nd else 60, - "max_concurrent_streams": nd.max_concurrent_streams if nd else 8, - "transcode_incompatible_video": nd.transcode_incompatible_video if nd else True, - "stun_servers": nd.stun_servers if nd and nd.stun_servers else list(DEFAULT_STUN_SERVERS), - "ice_interfaces": nd.ice_interfaces if nd else [], - } + from meshbay_node.config import node_settings_defaults + # No config (a test, an unconfigured node) falls back to NodeConfig()'s own + # values rather than to numbers repeated here, which is the copy this used + # to be: it was missing three settings and reported them as null. + defaults = node_settings_defaults(config.node if config else None) if roster: settings = await roster.node_settings(defaults) else: @@ -1288,24 +1283,33 @@ async def clear_denylist(state: dict, *, subject: str = "") -> dict: # ── Node settings ──────────────────────────────────────────────────────────── +# What `set_node_settings` accepts, and how each value is validated. A module +# constant so a test can hold its key set against `Roster.node_setting_keys()`: +# this is the third list of the same settings, and the first two had already +# drifted apart once — the reader's defaults covered fewer settings than the +# resolver answered for, which is how node.toml's transfer pools came to be +# parsed and then ignored. The kinds here are the *writer's* validation and +# deliberately not the resolver's coercions. +NODE_SETTING_WRITERS: dict[str, tuple[str, str]] = { + "invite_ttl_hours": ("int", Roster.SETTING_INVITE_TTL), + "pair_ttl_hours": ("int", Roster.SETTING_PAIR_TTL), + "device_request_ttl_minutes": ("int", Roster.SETTING_DEVICE_TTL), + "max_concurrent_streams": ("int", Roster.SETTING_MAX_STREAMS), + "max_concurrent_downloads": ("int", Roster.SETTING_MAX_DOWNLOADS), + "max_concurrent_uploads": ("int", Roster.SETTING_MAX_UPLOADS), + "max_upload_gb": ("size", Roster.SETTING_MAX_UPLOAD_GB), + "transcode_incompatible_video": ("bool", Roster.SETTING_TRANSCODE), + "stun_servers": ("stun_list", Roster.SETTING_STUN_SERVERS), + "ice_interfaces": ("list", Roster.SETTING_ICE_INTERFACES), +} + + async def get_node_settings(state: dict) -> dict: """Return current effective node settings.""" - from meshbay_node.config import DEFAULT_STUN_SERVERS + from meshbay_node.config import node_settings_defaults roster = _roster(state) config = _config(state) - nd = config.node - defaults = { - "invite_ttl_hours": nd.invite_ttl_hours, - "pair_ttl_hours": nd.pair_ttl_hours, - "device_request_ttl_minutes": nd.device_request_ttl_minutes, - "max_concurrent_streams": nd.max_concurrent_streams, - "max_concurrent_downloads": nd.max_concurrent_downloads, - "max_concurrent_uploads": nd.max_concurrent_uploads, - "max_upload_gb": nd.max_upload_gb, - "transcode_incompatible_video": nd.transcode_incompatible_video, - "stun_servers": nd.stun_servers if nd.stun_servers else list(DEFAULT_STUN_SERVERS), - "ice_interfaces": nd.ice_interfaces, - } + defaults = node_settings_defaults(config.node) if roster: return await roster.node_settings(defaults) return defaults @@ -1317,18 +1321,7 @@ async def set_node_settings(state: dict, settings: dict) -> dict: nd = config.node conf_path = Path(state.get("config_path") or DEFAULT_CONFIG_PATH) - allowed_keys = { - "invite_ttl_hours": ("int", roster.SETTING_INVITE_TTL), - "pair_ttl_hours": ("int", roster.SETTING_PAIR_TTL), - "device_request_ttl_minutes": ("int", roster.SETTING_DEVICE_TTL), - "max_concurrent_streams": ("int", roster.SETTING_MAX_STREAMS), - "max_concurrent_downloads": ("int", roster.SETTING_MAX_DOWNLOADS), - "max_concurrent_uploads": ("int", roster.SETTING_MAX_UPLOADS), - "max_upload_gb": ("size", roster.SETTING_MAX_UPLOAD_GB), - "transcode_incompatible_video": ("bool", roster.SETTING_TRANSCODE), - "stun_servers": ("stun_list", roster.SETTING_STUN_SERVERS), - "ice_interfaces": ("list", roster.SETTING_ICE_INTERFACES), - } + allowed_keys = NODE_SETTING_WRITERS set_by = state.get("node_user_id", "") updated = {} diff --git a/packages/meshbay-node/src/meshbay_node/roster.py b/packages/meshbay-node/src/meshbay_node/roster.py index 474db31..0753400 100644 --- a/packages/meshbay-node/src/meshbay_node/roster.py +++ b/packages/meshbay-node/src/meshbay_node/roster.py @@ -988,34 +988,55 @@ class Roster: SETTING_STUN_SERVERS = "stun_servers" SETTING_ICE_INTERFACES = "ice_interfaces" + # Every setting this resolver answers for, and how a stored string becomes + # a value. `node_settings` iterates these two and nothing else, and + # `config.node_settings_defaults` is built from the same names — so the + # defaults dict cannot quietly cover fewer settings than are resolved. + # + # It did. The daemon's dict was written by hand and omitted the two + # transfer pools, so on a node with no panel override they resolved to + # `defaults.get(key)` → None, were assigned back onto the config, and the + # transport skipped them: `node.toml` was parsed, validated, and then + # replaced by the transport's own defaults. A missing key is not an error + # anywhere along that path — it is a setting that stops working in silence, + # and invisible unless the operator picked a value other than the default. + NODE_SETTING_SCALARS: tuple[tuple[str, str, str], ...] = ( + ("invite_ttl_hours", SETTING_INVITE_TTL, "int"), + ("pair_ttl_hours", SETTING_PAIR_TTL, "int"), + ("device_request_ttl_minutes", SETTING_DEVICE_TTL, "int"), + ("max_concurrent_streams", SETTING_MAX_STREAMS, "int"), + ("max_concurrent_downloads", SETTING_MAX_DOWNLOADS, "int"), + ("max_concurrent_uploads", SETTING_MAX_UPLOADS, "int"), + ("transcode_incompatible_video", SETTING_TRANSCODE, "bool"), + ("max_upload_gb", SETTING_MAX_UPLOAD_GB, "float"), + ) + NODE_SETTING_LISTS: tuple[tuple[str, str], ...] = ( + ("stun_servers", SETTING_STUN_SERVERS), + ("ice_interfaces", SETTING_ICE_INTERFACES), + ) + + @classmethod + def node_setting_keys(cls) -> frozenset[str]: + """Every key `node_settings` returns — what a defaults dict must cover.""" + return frozenset([k for k, _, _ in cls.NODE_SETTING_SCALARS] + + [k for k, _ in cls.NODE_SETTING_LISTS]) + async def node_settings(self, defaults: dict) -> dict: """Current effective settings: roster override if present, else config default.""" import json as _json result = {} - for key, setting in [ - ("invite_ttl_hours", self.SETTING_INVITE_TTL), - ("pair_ttl_hours", self.SETTING_PAIR_TTL), - ("device_request_ttl_minutes", self.SETTING_DEVICE_TTL), - ("max_concurrent_streams", self.SETTING_MAX_STREAMS), - ("max_concurrent_downloads", self.SETTING_MAX_DOWNLOADS), - ("max_concurrent_uploads", self.SETTING_MAX_UPLOADS), - ("transcode_incompatible_video", self.SETTING_TRANSCODE), - ("max_upload_gb", self.SETTING_MAX_UPLOAD_GB), - ]: + for key, setting, kind in self.NODE_SETTING_SCALARS: stored = await self.get_setting(self.NODE_WIDE_GROUP_ID, setting) if stored is not None: - if key == "transcode_incompatible_video": + if kind == "bool": result[key] = stored != "0" - elif key == "max_upload_gb": + elif kind == "float": result[key] = float(stored) else: result[key] = int(stored) else: result[key] = defaults.get(key) - for list_key, setting in [ - ("stun_servers", self.SETTING_STUN_SERVERS), - ("ice_interfaces", self.SETTING_ICE_INTERFACES), - ]: + for list_key, setting in self.NODE_SETTING_LISTS: stored = await self.get_setting(self.NODE_WIDE_GROUP_ID, setting) if stored is not None: try: diff --git a/packages/meshbay-node/tests/test_node_settings_defaults.py b/packages/meshbay-node/tests/test_node_settings_defaults.py new file mode 100644 index 0000000..f5d7856 --- /dev/null +++ b/packages/meshbay-node/tests/test_node_settings_defaults.py @@ -0,0 +1,85 @@ +""" +Three lists name the same node settings, and they must agree. + +The resolver (`Roster.node_settings`) answers for a set of keys; the reader +(`config.node_settings_defaults`) supplies what node.toml says for each; the +writer (`ops.NODE_SETTING_WRITERS`) says which may be set and how each is +validated. Nothing in the code path errors when the reader covers fewer keys +than the resolver answers for: the missing one resolves to None, is written +back onto the config, and the consumer falls through to its own default. So +node.toml is parsed, validated — and ignored. + +That is what happened to `max_concurrent_downloads` and +`max_concurrent_uploads`, and it was invisible because the value it fell back +to was the same 8 the file suggests. Only an operator who set something else +would ever have seen it, and then only as pools that did not match the file. +""" +import pytest +from meshbay_node.config import NodeConfig, load_config, node_settings_defaults +from meshbay_node.ops import NODE_SETTING_WRITERS +from meshbay_node.roster import Roster + + +def test_the_reader_covers_every_setting_the_resolver_answers_for(): + assert set(node_settings_defaults()) == set(Roster.node_setting_keys()) + + +def test_the_writer_covers_every_setting_the_resolver_answers_for(): + assert set(NODE_SETTING_WRITERS) == set(Roster.node_setting_keys()) + + +def test_no_default_is_none(): + """A None here is indistinguishable from a key that is missing.""" + missing = [k for k, v in node_settings_defaults().items() if v is None] + assert not missing + + +@pytest.mark.asyncio +async def test_node_toml_survives_startup_with_no_panel_override(tmp_path): + """The whole bug, at the level an operator meets it.""" + conf = tmp_path / "node.toml" + conf.write_text( + '[hub]\nurl = "https://example.invalid"\nusername = "op"\n\n' + '[node]\n' + 'max_concurrent_downloads = 3\n' + 'max_concurrent_uploads = 2\n' + 'max_concurrent_streams = 5\n' + 'max_upload_gb = 4\n', + encoding="utf-8") + nd = load_config(conf).node + + roster = Roster(db_path=tmp_path / "roster.db") + await roster.open() + try: + effective = await roster.node_settings(node_settings_defaults(nd)) + finally: + await roster.close() + + assert effective["max_concurrent_downloads"] == 3 + assert effective["max_concurrent_uploads"] == 2 + assert effective["max_concurrent_streams"] == 5 + assert effective["max_upload_gb"] == 4.0 + + +@pytest.mark.asyncio +async def test_a_panel_override_still_wins_over_the_file(tmp_path): + conf = tmp_path / "node.toml" + conf.write_text( + '[hub]\nurl = "https://example.invalid"\nusername = "op"\n\n' + '[node]\nmax_concurrent_downloads = 3\n', encoding="utf-8") + nd = load_config(conf).node + + roster = Roster(db_path=tmp_path / "roster.db") + await roster.open() + try: + await roster.set_node_setting(Roster.SETTING_MAX_DOWNLOADS, "6", "op") + effective = await roster.node_settings(node_settings_defaults(nd)) + finally: + await roster.close() + + assert effective["max_concurrent_downloads"] == 6 + + +def test_an_absent_config_falls_back_to_the_dataclass_not_to_literals(): + """`node_status` used to repeat the numbers by hand, and they went stale.""" + assert node_settings_defaults(None) == node_settings_defaults(NodeConfig()) -- cgit v1.2.3