From 5214dd7a86a36581fd4dd685a3e1d2bb5a651eaa Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:08:06 +0300 Subject: [PATCH] fix: settings card export -- cap dict KEYS, not just values, in two more places Found while re-reading collect() after round 4's fix, same pattern as every prior round: an unaudited site where caller data reaches the card uncapped. - collect()'s per-input loop caps `value` via _short() but never capped `name` -- the input dict's KEY is caller data too (the whole `inputs` dict comes from the client), and flows verbatim into the drawn row label and the embedded JSON. - _rows_from_list's per-entry join (`f"{k}={_short(v)}"`) caps `v` but not `k` -- a huge key still gets fully materialized into the string before the joined result's own _short() call ever runs, so it's real per-row cost even though the final printed string comes out short. Both fixed with the same _cap() helper already used for slot id/group/node. Added regression tests for both. Co-Authored-By: Claude Sonnet 5 --- core/settings_card.py | 10 +++++++++- core/tests/test_settings_card.py | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/core/settings_card.py b/core/settings_card.py index 0401697e..51faf1c7 100644 --- a/core/settings_card.py +++ b/core/settings_card.py @@ -116,8 +116,12 @@ def _rows_from_list(value): # 1:1 with it) ballooned past 100MB. Also bound how many keys get # processed at all: a row with an absurd key count still costs one # f-string+join per key before any length cap ever applies. + # The KEY is caller data too, same as the value -- an uncapped key + # still gets fully materialized into `parts` before _short() on the + # joined result ever runs, so a huge key is real per-row cost even + # though the final string comes out short. items = list(row.items()) - parts = [f"{k}={_short(v)}" for k, v in items[:_MAX_KEYS_PER_LIST_ROW]] + parts = [f"{_cap(k)}={_short(v)}" for k, v in items[:_MAX_KEYS_PER_LIST_ROW]] if len(items) > _MAX_KEYS_PER_LIST_ROW: parts.append(f"…(+{len(items) - _MAX_KEYS_PER_LIST_ROW} more keys)") out.append((f"[{i}]", _short(" ".join(parts)) if parts else "—")) @@ -200,6 +204,10 @@ def collect(slots: list, host: dict, *, project_name=None, render=None) -> dict: break if name in _NOISE_INPUTS: continue + # `inputs` is a caller-controlled dict -- the KEY, not just the + # value, is attacker data. Every row below carries `name` + # verbatim into the drawn label and the embedded JSON. + name = _cap(name) if is_link(value): source = by_id.get(value[0]) source_label = _cap((source or {}).get("id") or value[0]) diff --git a/core/tests/test_settings_card.py b/core/tests/test_settings_card.py index b456a7d3..b0885363 100644 --- a/core/tests/test_settings_card.py +++ b/core/tests/test_settings_card.py @@ -228,6 +228,27 @@ def test_many_empty_input_slots_with_huge_ids_do_not_blow_up_the_embed(): assert len(embedded) < 2_000_000 +def test_a_huge_input_name_is_capped_not_embedded_whole(): + """The `inputs` dict's KEYS are caller data too, not just its values -- + found during round-4 cleanup by re-reading collect()'s row loop, not by + an attacker report. Every row carries `name` verbatim into the drawn + label and the embedded JSON, so it needs the same cap as any value.""" + rep = sc.collect([{"id": "s", "node": "X", "inputs": {"k" * 50_000: "v"}}], HOST) + key = rep["sections"][0]["rows"][0][0] + assert len(key) < 500 + + +def test_a_huge_funpack_list_dict_key_is_capped_not_just_its_value(): + """Same gap, one level deeper: _rows_from_list joins `f"{k}={_short(v)}"` + per dict key in a funpack_list row -- _short() bounded v but not k, so a + huge key still got fully materialized into the join before the final + _short() on the joined string ever ran.""" + rows = json.dumps([{("k" * 500_000): "v"}]) + rep = sc.collect([{"id": "l", "node": "X", "inputs": {"lora_list": rows}}], HOST) + text = " ".join(v for _, v in rep["sections"][0]["rows"]) + assert len(text) < 2000 + + def test_render_png_stays_fast_when_one_slot_has_many_large_lists(): """The Finding B composition gap, carried all the way through to the actual render (67.8s / 123MB before the fix)."""