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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
eda757e103
commit
5214dd7a86
@@ -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])
|
||||
|
||||
@@ -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)."""
|
||||
|
||||
Reference in New Issue
Block a user